Add the NONZERO bytecode and generate it appropriately

src/codegen/ast_interpreter.cpp

@@ -403,7 +403,10 @@ Value ASTInterpreter::visit_slice(AST_Slice* node) {
 }
 
 Value ASTInterpreter::visit_branch(AST_Branch* node) {
-    if (nonzero(visit_expr(node->test).o))
+    Value v = visit_expr(node->test);
+    ASSERT(v.n == 0 || v.n == 1, "Should have called NONZERO before this branch");
+
+    if (v.b)
         next_block = node->iftrue;
     else
         next_block = node->iffalse;
@@ -441,6 +444,8 @@ Value ASTInterpreter::visit_jump(AST_Jump* node) {
                 if (phis->isPotentiallyUndefinedAfter(name, current_block)) {
                     bool is_defined = it != sym_table.end();
                     sorted_symbol_table[getIsDefinedName(name)] = (Box*)is_defined;
+                    if (is_defined)
+                        assert(it->getValue() != NULL);
                     sorted_symbol_table[name] = is_defined ? it->getValue() : NULL;
                 } else {
                     ASSERT(it != sym_table.end(), "%s", name.c_str());
@@ -567,6 +572,7 @@ Value ASTInterpreter::visit_langPrimitive(AST_LangPrimitive* node) {
         v = boxBool(isinstance(obj.o, cls.o, unboxInt(flags.o)));
 
     } else if (node->opcode == AST_LangPrimitive::LOCALS) {
+        assert(node->args.size() == 0);
         BoxedDict* dict = new BoxedDict;
         for (auto& p : sym_table) {
             llvm::StringRef s = p.first();
@@ -576,6 +582,12 @@ Value ASTInterpreter::visit_langPrimitive(AST_LangPrimitive* node) {
             dict->d[new BoxedString(s.str())] = p.second;
         }
         v = dict;
+    } else if (node->opcode == AST_LangPrimitive::NONZERO) {
+        assert(node->args.size() == 1);
+        Value obj = visit_expr(node->args[0]);
+        // TODO we could not add the int64_t, which would only set the lower byte of v,
+        // since that's all we're going to look at later.
+        v = (int64_t)nonzero(obj.o);
     } else
         RELEASE_ASSERT(0, "not implemented");
     return v;
@@ -735,8 +747,13 @@ Value ASTInterpreter::visit_raise(AST_Raise* node) {
 }
 
 Value ASTInterpreter::visit_assert(AST_Assert* node) {
-    if (!nonzero(visit_expr(node->test).o))
-        assertFail(source_info->parent_module, node->msg ? visit_expr(node->msg).o : 0);
+#ifndef NDEBUG
+    // Currently we only generate "assert 0" statements
+    Value v = visit_expr(node->test);
+    assert(v.n == 0);
+#endif
+    assertFail(source_info->parent_module, node->msg ? visit_expr(node->msg).o : 0);
+
     return Value();
 }
 
@@ -1002,8 +1019,6 @@ Value ASTInterpreter::visit_name(AST_Name* node) {
         SymMap::iterator it = sym_table.find(node->id);
         if (it != sym_table.end()) {
             Box* value = it->second;
-            if (!value)
-                assertNameDefined(value, node->id.c_str(), UnboundLocalError, true);
             return value;
         }
 

src/codegen/irgen/irgenerator.cpp

@@ -1812,22 +1812,18 @@ private:
         assert(state != PARTIAL);
         assert(val);
 
-        // ASSERT(val->getType() == BOOL, "%s", val->getType()->debugName().c_str());
+        // We could call nonzero here if there is no try-catch block?
+        ASSERT(val->getType() == BOOL, "should have called NONZERO before this; is %s",
+               val->getType()->debugName().c_str());
+        llvm::Value* v = i1FromBool(emitter, static_cast<ConcreteCompilerVariable*>(val));
+        assert(v->getType() == g.i1);
 
-        ConcreteCompilerVariable* nonzero = val->nonzero(emitter, getOpInfoForNode(node, unw_info));
-        ASSERT(nonzero->getType() == BOOL, "%s %s", val->getType()->debugName().c_str(),
-               nonzero->getType()->debugName().c_str());
-        val->decvref(emitter);
-
-        llvm::Value* llvm_nonzero = i1FromBool(emitter, nonzero);
         llvm::BasicBlock* iftrue = entry_blocks[node->iftrue];
         llvm::BasicBlock* iffalse = entry_blocks[node->iffalse];
 
-        nonzero->decvref(emitter);
-
         endBlock(FINISHED);
 
-        emitter.getBuilder()->CreateCondBr(llvm_nonzero, iftrue, iffalse);
+        emitter.getBuilder()->CreateCondBr(v, iftrue, iffalse);
     }
 
     void doExpr(AST_Expr* node, UnwindInfo unw_info) {

src/core/ast.cpp

@@ -24,6 +24,10 @@
 
 namespace pyston {
 
+#ifndef NDEBUG
+int AST::next_lineno = 100000;
+#endif
+
 llvm::StringRef getOpSymbol(int op_type) {
     switch (op_type) {
         case AST_TYPE::Add:
@@ -1448,6 +1452,9 @@ bool PrintVisitor::visit_langprimitive(AST_LangPrimitive* node) {
         case AST_LangPrimitive::NONE:
             printf("NONE");
             break;
+        case AST_LangPrimitive::NONZERO:
+            printf("NONZERO");
+            break;
         default:
             RELEASE_ASSERT(0, "%d", node->opcode);
     }

src/core/ast.h

@@ -148,7 +148,17 @@ public:
 
     virtual void accept(ASTVisitor* v) = 0;
 
+#ifndef NDEBUG
+private:
+    static int next_lineno;
+
+public:
+    // In debug mode, initialize lineno to something unique, so that if we see something ridiculous
+    // appear in the traceback, we can isolate the allocation which created it.
+    AST(AST_TYPE::AST_TYPE type) : type(type), lineno(++next_lineno) {}
+#else
     AST(AST_TYPE::AST_TYPE type) : type(type) {}
+#endif
 };
 
 class AST_expr : public AST {
@@ -966,6 +976,7 @@ public:
         IMPORT_NAME,
         IMPORT_STAR,
         NONE,
+        NONZERO,
     } opcode;
     std::vector<AST_expr*> args;
 

src/core/cfg.cpp

@@ -131,6 +131,17 @@ private:
         return NULL;
     }
 
+    AST_expr* callNonzero(AST_expr* e) {
+        AST_LangPrimitive* call = new AST_LangPrimitive(AST_LangPrimitive::NONZERO);
+        call->args.push_back(e);
+        call->lineno = e->lineno;
+        call->col_offset = e->col_offset;
+
+        auto name = nodeName(e);
+        pushAssign(name, call);
+        return makeName(name, AST_TYPE::Load);
+    }
+
     AST_expr* applyComprehensionCall(AST_DictComp* node, AST_Name* name) {
         AST_expr* key = remapExpr(node->key);
         AST_expr* value = remapExpr(node->value);
@@ -179,7 +190,7 @@ private:
             push_back(j);
 
             curblock = test_block;
-            AST_expr* test_call = remapExpr(makeCall(hasnext_attr));
+            AST_expr* test_call = callNonzero(remapExpr(makeCall(hasnext_attr)));
 
             CFGBlock* body_block = cfg->addBlock();
             body_block->info = "comprehension_body";
@@ -204,7 +215,7 @@ private:
             pushAssign(c->target, makeName(next_name, AST_TYPE::Load));
 
             for (AST_expr* if_condition : c->ifs) {
-                AST_expr* remapped = remapExpr(if_condition);
+                AST_expr* remapped = callNonzero(remapExpr(if_condition));
                 AST_Branch* br = new AST_Branch();
                 br->test = remapped;
                 push_back(br);
@@ -284,7 +295,7 @@ private:
 
     AST_Branch* makeBranch(AST_expr* test) {
         AST_Branch* rtn = new AST_Branch();
-        rtn->test = test;
+        rtn->test = callNonzero(test);
         rtn->col_offset = test->col_offset;
         rtn->lineno = test->lineno;
         return rtn;
@@ -520,7 +531,7 @@ private:
             pushAssign(name, val);
 
             AST_Branch* br = new AST_Branch();
-            br->test = _dup(val);
+            br->test = callNonzero(_dup(val));
             push_back(br);
 
             CFGBlock* was_block = curblock;
@@ -637,7 +648,7 @@ private:
                 pushAssign(name, val);
 
                 AST_Branch* br = new AST_Branch();
-                br->test = makeName(name, AST_TYPE::Load);
+                br->test = callNonzero(makeName(name, AST_TYPE::Load));
                 push_back(br);
 
                 CFGBlock* was_block = curblock;
@@ -725,7 +736,7 @@ private:
 
             for (AST_expr* if_condition : c->ifs) {
                 AST_If* if_block = new AST_If();
-                if_block->test = if_condition;
+                if_block->test = callNonzero(if_condition);
 
                 insert_point->push_back(if_block);
                 insert_point = &if_block->body;
@@ -751,13 +762,14 @@ private:
     AST_expr* remapIfExp(AST_IfExp* node) {
         std::string rtn_name = nodeName(node);
 
-        CFGBlock* starting_block = curblock;
         AST_Branch* br = new AST_Branch();
         br->col_offset = node->col_offset;
         br->lineno = node->lineno;
-        br->test = remapExpr(node->test);
+        br->test = callNonzero(remapExpr(node->test));
         push_back(br);
 
+        CFGBlock* starting_block = curblock;
+
         CFGBlock* iftrue = cfg->addBlock();
         iftrue->info = "iftrue";
         br->iftrue = iftrue;
@@ -1250,7 +1262,7 @@ public:
 
     bool visit_assert(AST_Assert* node) override {
         AST_Branch* br = new AST_Branch();
-        br->test = remapExpr(node->test);
+        br->test = callNonzero(remapExpr(node->test));
         push_back(br);
 
         CFGBlock* iffalse = cfg->addBlock();
@@ -1502,7 +1514,7 @@ public:
         AST_Branch* br = new AST_Branch();
         br->col_offset = node->col_offset;
         br->lineno = node->lineno;
-        br->test = remapExpr(node->test);
+        br->test = callNonzero(remapExpr(node->test));
         push_back(br);
 
         CFGBlock* starting_block = curblock;
@@ -1831,7 +1843,7 @@ public:
                     is_caught_here->args.push_back(makeNum(1)); // flag: false_on_noncls
 
                     AST_Branch* br = new AST_Branch();
-                    br->test = remapExpr(is_caught_here);
+                    br->test = callNonzero(remapExpr(is_caught_here));
 
                     CFGBlock* exc_handle = cfg->addBlock();
                     exc_next = cfg->addDeferredBlock();
@@ -2157,8 +2169,18 @@ CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body) {
     for (auto b : rtn->blocks)
         flatten(b->body, flattened, true);
 
-    std::unordered_set<AST*> deduped(flattened.begin(), flattened.end());
-    assert(deduped.size() == flattened.size());
+    std::unordered_map<AST*, int> deduped;
+    bool no_dups = true;
+    for (auto e : flattened) {
+        deduped[e]++;
+        if (deduped[e] == 2) {
+            printf("Duplicated: ");
+            print_ast(e);
+            printf("\n");
+            no_dups = false;
+        }
+    }
+    assert(no_dups);
 
 // TODO make sure the result of Invoke nodes are not used on the exceptional path
 #endif

src/runtime/objmodel.cpp

@@ -1631,6 +1631,8 @@ bool isUserDefined(BoxedClass* cls) {
 }
 
 extern "C" bool nonzero(Box* obj) {
+    assert(gc::isValidGCObject(obj));
+
     static StatCounter slowpath_nonzero("slowpath_nonzero");
 
     std::unique_ptr<Rewriter> rewriter(

test/tests/exception_subclasscheck.py

@@ -3,6 +3,9 @@
 
 # Exception-catching is supposed to go through __subclasscheck__
 
+class MyException(Exception):
+    pass
+
 class M(type):
     def __instancecheck__(self, instance):
         print "instancecheck", instance
@@ -10,10 +13,19 @@ class M(type):
 
     def __subclasscheck__(self, sub):
         print "subclasscheck", sub
+
+        if self.throw_on_subclasscheck:
+            raise MyException()
+
         return True
 
 class E(Exception):
     __metaclass__ = M
+    throw_on_subclasscheck = False
+
+class F(Exception):
+    __metaclass__ = M
+    throw_on_subclasscheck = True
 
 print 1
 print isinstance(E(), E) # does not print anything due to special-casing
@@ -35,3 +47,12 @@ except E:
     pass
 
 print 5
+
+# Exceptions in __subclasscheck__ should get ignored:
+try:
+    1/0
+except F:
+    print "shouldn't get here"
+except ZeroDivisionError:
+    print "ok"
+

test/tests/nonzero_exceptions.py

@@ -6,48 +6,50 @@ class C(object):
         self.x = x
     def __nonzero__(self):
         raise MyException(self.x)
+    def __repr__(self):
+        return "<C %r>" % self.x
 
 # Make sure that we can handle nonzero() throwing an exception wherever it occurs:
 try:
     print C(1) and 1
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     print C(2) or 1
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     if C(3):
         pass
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     while C(4):
         pass
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     assert C(5)
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     print [1 for i in range(5) if C(6)]
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     print list(1 for i in range(5) if C(7))
-except TypeError, e:
+except MyException, e:
     print e
 
 try:
     print 1 if C(8) else 0
-except TypeError, e:
+except MyException, e:
     print e
 
 
@@ -65,25 +67,13 @@ class M(type):
 class F(Exception):
     __metaclass__ = M
 
-# We should check the type of exception against F using __subclasscheck__, but it's going throw an exception
-# trying to evaluate __subclasscheck__.
-# But that new error should get suppressed and the original exception not caught (by that block)
-try:
-    1/0
-except F:
-    print "shouldn't get here"
-except ZeroDivisionError:
-    print "ok"
-
-# But if the exception gets thrown at a slightly different point in the
-# exception-handler-filtering, the new exception propagates!
 try:
     try:
         1/0
-    except C() or 1:
+    except C("a") or 1:
         print "shouldn't get here"
     print "shouldn't get here 2"
-except TypeError, e:
+except MyException, e:
     print e
 
 
@@ -96,5 +86,5 @@ print E() < 1
 try:
     # This is not ok because it will!
     print E() < 1 < 1
-except TypeError, e:
+except MyException, e:
     print e