Getattr guarding issue

Wasn't checking the return convention -- not sure how I missed this
when I added that.

src/core/cfg.h

@@ -67,6 +67,9 @@ public:
 
     void push_back(AST_stmt* node) { body.push_back(node); }
     void print(llvm::raw_ostream& stream = llvm::outs());
+    void _print() {
+        print();
+    }
 };
 
 // Control Flow Graph

src/runtime/builtin_modules/builtins.cpp

@@ -564,14 +564,18 @@ Box* getattrFuncInternal(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args,
     if (rewrite_args) {
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, rewrite_args->arg1, rewrite_args->destination);
         rtn = getattrInternal<CAPI>(obj, str, &grewrite_args);
-        if (!grewrite_args.out_success)
+        // TODO could make the return valid in the NOEXC_POSSIBLE case via a helper
+        if (!grewrite_args.out_success || grewrite_args.out_return_convention == GetattrRewriteArgs::NOEXC_POSSIBLE)
             rewrite_args = NULL;
         else {
-            if (!rtn && !PyErr_Occurred())
+            if (!rtn && !PyErr_Occurred()) {
+                assert(grewrite_args.out_return_convention == GetattrRewriteArgs::NO_RETURN);
                 r_rtn = rewrite_args->rewriter->loadConst(0);
-            else
+            } else {
+                assert(grewrite_args.out_return_convention == GetattrRewriteArgs::VALID_RETURN);
                 r_rtn = grewrite_args.out_rtn;
             }
+        }
     } else {
         rtn = getattrInternal<CAPI>(obj, str, NULL);
     }
@@ -673,14 +677,17 @@ Box* hasattrFuncInternal(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args,
     if (rewrite_args) {
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, rewrite_args->arg1, rewrite_args->destination);
         rtn = getattrInternal<CAPI>(obj, str, &grewrite_args);
-        if (!grewrite_args.out_success)
+        if (!grewrite_args.out_success || grewrite_args.out_return_convention == GetattrRewriteArgs::NOEXC_POSSIBLE)
             rewrite_args = NULL;
         else {
-            if (!rtn && !PyErr_Occurred())
+            if (!rtn && !PyErr_Occurred()) {
+                assert(grewrite_args.out_return_convention == GetattrRewriteArgs::NO_RETURN);
                 r_rtn = rewrite_args->rewriter->loadConst(0);
-            else
+            } else {
+                assert(grewrite_args.out_return_convention == GetattrRewriteArgs::VALID_RETURN);
                 r_rtn = grewrite_args.out_rtn;
             }
+        }
     } else {
         rtn = getattrInternal<CAPI>(obj, str, NULL);
     }
@@ -1322,8 +1329,8 @@ Box* getreversed(Box* o) {
     return new (seqreviter_cls) BoxedSeqIter(o, len - 1);
 }
 
-Box* pydump(Box* p) {
-    dump(p);
+Box* pydump(Box* p, BoxedInt* level) {
+    dumpEx(p, level->n);
     return None;
 }
 
@@ -1587,8 +1594,8 @@ void setupBuiltins() {
     builtins_module->giveAttr("ord", ord_obj);
     trap_obj = new BoxedBuiltinFunctionOrMethod(boxRTFunction((void*)trap, UNKNOWN, 0), "trap");
     builtins_module->giveAttr("trap", trap_obj);
-    builtins_module->giveAttr("dump",
-                              new BoxedBuiltinFunctionOrMethod(boxRTFunction((void*)pydump, UNKNOWN, 1), "dump"));
+    builtins_module->giveAttr(
+        "dump", new BoxedBuiltinFunctionOrMethod(boxRTFunction((void*)pydump, UNKNOWN, 2), "dump", { boxInt(0) }));
     builtins_module->giveAttr(
         "dumpAddr", new BoxedBuiltinFunctionOrMethod(boxRTFunction((void*)pydumpAddr, UNKNOWN, 1), "dumpAddr"));
 

src/runtime/util.cpp

@@ -15,6 +15,7 @@
 #include "runtime/util.h"
 
 #include "codegen/codegen.h"
+#include "core/cfg.h"
 #include "core/options.h"
 #include "core/types.h"
 #include "runtime/long.h"
@@ -258,6 +259,11 @@ extern "C" void dumpEx(void* p, int levels) {
             CLFunction* cl = f->f;
             if (cl->source) {
                 printf("User-defined function '%s'\n", cl->source->getName()->c_str());
+                printf("Defined at %s:%d\n", cl->source->getFn()->c_str(), cl->source->body[0]->lineno);
+
+                if (cl->source->cfg && levels > 0) {
+                    cl->source->cfg->print();
+                }
             } else {
                 printf("A builtin function\n");
             }

test/tests/hasattr_guarding.py → test/tests/getattr_guarding.py

-# Test some weird hasattr guarding scenarios
-# I think this applies equally well to getattr
+# Test some weird getattr/hasattr guarding scenarios
 
+
+# Make sure that we guard correctly when a __getattr__ is involved:
 def f(o):
     print hasattr(o, "a")
     print getattr(o, "a", None)
@@ -21,3 +22,14 @@ for i in xrange(300):
     setattr(c, str(i), i)
     f(c)
 
+
+# Make sure that we guard correctly when we are megamorphic
+class C(object):
+    pass
+l = []
+for i in xrange(200):
+    c = C()
+    setattr(c, "a%d" % i, i)
+
+    # We should do the right guarding so that when i==150 comes around, this will return True/None:
+    print i, hasattr(c, "a150"), getattr(c, "a150", None)