'del name' support

Ended up not being too bad, except for some more fun around classdefs
having different rules, and making sure to generate the correct error
type and messages.

src/analysis/function_analysis.cpp

@@ -76,7 +76,7 @@ public:
     bool visit_name(AST_Name* node) {
         if (node->ctx_type == AST_TYPE::Load)
             _doLoad(node->id);
-        else if (node->ctx_type == AST_TYPE::Store)
+        else if (node->ctx_type == AST_TYPE::Store || node->ctx_type == AST_TYPE::Del)
             _doStore(node->id);
         else {
             assert(0);

src/analysis/scoping_analysis.cpp

@@ -155,6 +155,7 @@ public:
                 break;
             case AST_TYPE::Param:
             case AST_TYPE::Store:
+            case AST_TYPE::Del:
                 doWrite(node->id);
                 break;
             default:
@@ -212,7 +213,9 @@ public:
 
     virtual bool visit_delete(AST_Delete* node) {
         for (auto t : node->targets) {
-            RELEASE_ASSERT(t->type != AST_TYPE::Name, "");
+            if (t->type == AST_TYPE::Name) {
+                doWrite(ast_cast<AST_Name>(t)->id);
+            }
         }
         return false;
     }

src/analysis/type_analysis.cpp

@@ -482,8 +482,11 @@ private:
                 case AST_TYPE::Attribute:
                     getType(ast_cast<AST_Attribute>(target)->value);
                     break;
+                case AST_TYPE::Name:
+                    sym_table.erase(ast_cast<AST_Name>(target)->id);
+                    break;
                 default:
-                    RELEASE_ASSERT(0, "");
+                    RELEASE_ASSERT(0, "%d", target->type);
             }
         }
     }

src/codegen/irgen/irgenerator.cpp

@@ -667,8 +667,10 @@ private:
 
                 // TODO should mark as DEAD here, though we won't end up setting all the names appropriately
                 // state = DEAD;
-                llvm::CallSite call = emitter.createCall2(exc_info, g.funcs.assertNameDefined, getConstantInt(0, g.i1),
-                                                          getStringConstantPtr(node->id + '\0'));
+                llvm::CallSite call = emitter.createCall(
+                    exc_info, g.funcs.assertNameDefined,
+                    { getConstantInt(0, g.i1), getStringConstantPtr(node->id + '\0'),
+                      embedConstantPtr(UnboundLocalError, g.llvm_class_type_ptr), getConstantInt(true, g.i1) });
                 call.setDoesNotReturn();
                 return undefVariable();
             }
@@ -689,16 +691,19 @@ private:
                     emitter.getBuilder()->CreateCondBr(is_defined_var->getValue(), from_local, from_global);
 
                     emitter.getBuilder()->SetInsertPoint(from_local);
+                    curblock = from_local;
                     CompilerVariable* local = symbol_table[node->id];
                     ConcreteCompilerVariable* converted_local = local->makeConverted(emitter, local->getBoxType());
                     // don't decvref local here, because are manufacturing a new vref
                     emitter.getBuilder()->CreateBr(join);
 
                     emitter.getBuilder()->SetInsertPoint(from_global);
+                    curblock = from_global;
                     ConcreteCompilerVariable* global = _getGlobal(node, exc_info);
                     emitter.getBuilder()->CreateBr(join);
 
                     emitter.getBuilder()->SetInsertPoint(join);
+                    curblock = join;
                     llvm::PHINode* phi = emitter.getBuilder()->CreatePHI(g.llvm_value_type_ptr, 2, node->id);
                     phi->addIncoming(converted_local->getValue(), from_local);
                     phi->addIncoming(global->getValue(), from_global);
@@ -706,8 +711,10 @@ private:
                     return new ConcreteCompilerVariable(UNKNOWN, phi, true);
                 }
 
-                emitter.createCall2(exc_info, g.funcs.assertNameDefined, is_defined_var->getValue(),
-                                    getStringConstantPtr(node->id + '\0'));
+                emitter.createCall(exc_info, g.funcs.assertNameDefined,
+                                   { is_defined_var->getValue(), getStringConstantPtr(node->id + '\0'),
+                                     embedConstantPtr(UnboundLocalError, g.llvm_class_type_ptr),
+                                     getConstantInt(true, g.i1) });
 
                 // At this point we know the name must be defined (otherwise the assert would have fired):
                 _popFake(defined_name);
@@ -1306,8 +1313,11 @@ private:
                 case AST_TYPE::Attribute:
                     _doDelAttr(static_cast<AST_Attribute*>(target), exc_info);
                     break;
+                case AST_TYPE::Name:
+                    _doDelName(static_cast<AST_Name*>(target), exc_info);
+                    break;
                 default:
-                    ASSERT(0, "UnSupported del target: %d", target->type);
+                    ASSERT(0, "Unsupported del target: %d", target->type);
                     abort();
             }
         }
@@ -1348,6 +1358,46 @@ private:
         value->delattr(emitter, getEmptyOpInfo(exc_info), &node->attr);
     }
 
+    void _doDelName(AST_Name* target, ExcInfo exc_info) {
+        auto scope_info = irstate->getScopeInfo();
+        if (scope_info->refersToGlobal(target->id)) {
+            // Can't use delattr since the errors are different:
+            emitter.createCall2(exc_info, g.funcs.delGlobal,
+                                embedConstantPtr(irstate->getSourceInfo()->parent_module, g.llvm_module_type_ptr),
+                                embedConstantPtr(&target->id, g.llvm_str_type_ptr));
+            return;
+        }
+
+        assert(!scope_info->refersToClosure(target->id));
+        assert(!scope_info->saveInClosure(
+            target->id)); // SyntaxError: can not delete variable 'x' referenced in nested scope
+
+        // A del of a missing name generates different error messages in a function scope vs a classdef scope
+        bool local_error_msg = (irstate->getSourceInfo()->ast->type != AST_TYPE::ClassDef);
+
+        if (symbol_table.count(target->id) == 0) {
+            llvm::CallSite call = emitter.createCall(exc_info, g.funcs.assertNameDefined,
+                                                     { getConstantInt(0, g.i1), getStringConstantPtr(target->id + '\0'),
+                                                       embedConstantPtr(NameError, g.llvm_class_type_ptr),
+                                                       getConstantInt(local_error_msg, g.i1) });
+            call.setDoesNotReturn();
+            return;
+        }
+
+        std::string defined_name = _getFakeName("is_defined", target->id.c_str());
+        ConcreteCompilerVariable* is_defined_var = static_cast<ConcreteCompilerVariable*>(_getFake(defined_name, true));
+
+        if (is_defined_var) {
+            emitter.createCall(exc_info, g.funcs.assertNameDefined,
+                               { is_defined_var->getValue(), getStringConstantPtr(target->id + '\0'),
+                                 embedConstantPtr(NameError, g.llvm_class_type_ptr),
+                                 getConstantInt(local_error_msg, g.i1) });
+            _popFake(defined_name);
+        }
+
+        symbol_table.erase(target->id);
+    }
+
     CLFunction* _wrapFunction(AST* node, AST_arguments* args, const std::vector<AST_stmt*>& body) {
         // Different compilations of the parent scope of a functiondef should lead
         // to the same CLFunction* being used:

src/codegen/runtime_hooks.cpp

@@ -174,6 +174,7 @@ void initGlobalFuncs(GlobalState& g) {
     GET(setitem);
     GET(delitem);
     GET(getGlobal);
+    GET(delGlobal);
     GET(binop);
     GET(compare);
     GET(augbinop);

src/codegen/runtime_hooks.h

@@ -33,8 +33,9 @@ struct GlobalFuncs {
     llvm::Value* boxInt, *unboxInt, *boxFloat, *unboxFloat, *boxStringPtr, *boxCLFunction, *unboxCLFunction,
         *boxInstanceMethod, *boxBool, *unboxBool, *createTuple, *createDict, *createList, *createSlice,
         *createUserClass, *createClosure;
-    llvm::Value* getattr, *setattr, *delattr, *print, *nonzero, *binop, *compare, *augbinop, *unboxedLen, *getitem,
-        *getclsattr, *getGlobal, *setitem, *delitem, *unaryop, *import, *importFrom, *importStar, *repr, *isinstance;
+    llvm::Value* getattr, *setattr, *delattr, *delitem, *delGlobal, *print, *nonzero, *binop, *compare, *augbinop,
+        *unboxedLen, *getitem, *getclsattr, *getGlobal, *setitem, *unaryop, *import, *importFrom, *importStar, *repr,
+        *isinstance;
 
     llvm::Value* checkUnpackingLength, *raiseAttributeError, *raiseAttributeErrorStr, *raiseNotIterableError,
         *assertNameDefined, *assertFail;

src/core/cfg.cpp

@@ -1093,9 +1093,12 @@ public:
                     target = astattr;
                     break;
                 }
-
+                case AST_TYPE::Name: {
+                    target = t;
+                    break;
+                }
                 default:
-                    RELEASE_ASSERT(0, "UnSupported del target: %d", t->type);
+                    RELEASE_ASSERT(0, "Unsupported del target: %d", t->type);
             }
             astdel->targets.push_back(target);
             push_back(astdel);

src/runtime/inline/link_forcer.cpp

@@ -71,6 +71,7 @@ void force() {
     FORCE(getitem);
     FORCE(getclsattr);
     FORCE(getGlobal);
+    FORCE(delGlobal);
     FORCE(setitem);
     FORCE(delitem);
     FORCE(unaryop);

src/runtime/objmodel.cpp

@@ -250,9 +250,12 @@ extern "C" void assertFail(BoxedModule* inModule, Box* msg) {
     }
 }
 
-extern "C" void assertNameDefined(bool b, const char* name) {
+extern "C" void assertNameDefined(bool b, const char* name, BoxedClass* exc_cls, bool local_var_msg) {
     if (!b) {
-        raiseExcHelper(UnboundLocalError, "local variable '%s' referenced before assignment", name);
+        if (local_var_msg)
+            raiseExcHelper(exc_cls, "local variable '%s' referenced before assignment", name);
+        else
+            raiseExcHelper(exc_cls, "name '%s' is not defined", name);
     }
 }
 
@@ -2659,6 +2662,7 @@ void Box::delattr(const std::string& attr, DelattrRewriteArgs2* rewrite_args) {
     // of remaining attributes
     int num_attrs = hcls->attr_offsets.size();
     int offset = hcls->getOffset(attr);
+    assert(offset >= 0);
     Box** start = attrs->attr_list->attrs;
     memmove(start + offset, start + offset + 1, (num_attrs - offset - 1) * sizeof(Box*));
 
@@ -2945,6 +2949,13 @@ Box* typeNew(Box* cls, Box* obj) {
     return rtn;
 }
 
+extern "C" void delGlobal(BoxedModule* m, std::string* name) {
+    if (!m->getattr(*name)) {
+        raiseExcHelper(NameError, "name '%s' is not defined", name->c_str());
+    }
+    m->delattr(*name, NULL);
+}
+
 extern "C" Box* getGlobal(BoxedModule* m, std::string* name) {
     static StatCounter slowpath_getglobal("slowpath_getglobal");
     slowpath_getglobal.log();

src/runtime/objmodel.h

@@ -65,6 +65,7 @@ extern "C" i64 unboxedLen(Box* obj);
 extern "C" Box* binop(Box* lhs, Box* rhs, int op_type);
 extern "C" Box* augbinop(Box* lhs, Box* rhs, int op_type);
 extern "C" Box* getGlobal(BoxedModule* m, std::string* name);
+extern "C" void delGlobal(BoxedModule* m, std::string* name);
 extern "C" Box* getitem(Box* value, Box* slice);
 extern "C" void setitem(Box* target, Box* slice, Box* value);
 extern "C" void delitem(Box* target, Box* slice);
@@ -74,7 +75,7 @@ extern "C" Box* import(const std::string* name);
 extern "C" Box* importFrom(Box* obj, const std::string* attr);
 extern "C" void importStar(Box* from_module, BoxedModule* to_module);
 extern "C" void checkUnpackingLength(i64 expected, i64 given);
-extern "C" void assertNameDefined(bool b, const char* name);
+extern "C" void assertNameDefined(bool b, const char* name, BoxedClass* exc_cls, bool local_var_msg);
 extern "C" void assertFail(BoxedModule* inModule, Box* msg);
 extern "C" bool isSubclass(BoxedClass* child, BoxedClass* parent);
 extern "C" BoxedClosure* createClosure(BoxedClosure* parent_closure);

test/tests/del_name.py

+import sys
+
+def p():
+    print hasattr(sys.modules['__main__'], 'a')
+
+p()
+a = 1
+p()
+del a
+p()
+try:
+    del a
+except NameError, e:
+    print e
+
+
+a = 1
+def mk_cls(b1, b2):
+    class C(object):
+        if b1:
+            a = 2
+        if b2:
+            # with b1==False and b2==True, this reference to 'a' will go to the global scope.
+            # The 'del a' will still refer to the local scope and raise a NameError.
+            print a
+            del a
+    print hasattr(C, 'a')
+
+mk_cls(False, False)
+mk_cls(True, False)
+mk_cls(True, True)
+try:
+    mk_cls(False, True)
+    assert 0
+except NameError, e:
+    print e
+
+
+def f1(b1, b2):
+    if b1:
+        a = 2
+    if b2:
+        del a
+
+f1(False, False)
+f1(True, False)
+f1(True, True)
+try:
+    f1(False, True)
+    assert 0
+except NameError, e:
+    print e
+
+def f2():
+    del a
+try:
+    f2()
+    assert 0
+except NameError, e:
+    print e

test/tests/del_name_scoping.py

-# expected: fail
-# - deletes on names
-
 x = 1
 def f():
     if 0: