add MakeClass, MakeFunction AST psuedo-exprs

This lets class & function definitions become ordinary assignments in
the CFG, which simplifies our invariants and fixes problems with the
interaction between definedness analysis and block-terminating
"invoke"s.

src/analysis/type_analysis.cpp

@@ -511,7 +511,9 @@ private:
         }
     }
 
-    void visit_classdef(AST_ClassDef* node) override {
+    void* visit_makeclass(AST_MakeClass* mkclass) override {
+        AST_ClassDef* node = mkclass->class_def;
+
         for (auto d : node->decorator_list) {
             getType(d);
         }
@@ -521,9 +523,8 @@ private:
         }
 
         // TODO should we speculate that classdefs will generally return a class?
-        // CompilerType* t = typeFromClass(type_cls);
-        CompilerType* t = UNKNOWN;
-        _doSet(scope_info->mangleName(node->name), t);
+        // return typeFromClass(type_cls);
+        return UNKNOWN;
     }
 
     void visit_delete(AST_Delete* node) override {
@@ -551,7 +552,9 @@ private:
         }
     }
 
-    void visit_functiondef(AST_FunctionDef* node) override {
+    void* visit_makefunction(AST_MakeFunction* mkfn) override {
+        AST_FunctionDef* node = mkfn->function_def;
+
         for (auto d : node->decorator_list) {
             getType(d);
         }
@@ -563,7 +566,7 @@ private:
         CompilerType* t = UNKNOWN;
         if (node->decorator_list.empty())
             t = typeFromClass(function_cls);
-        _doSet(scope_info->mangleName(node->name), t);
+        return t;
     }
 
     void visit_global(AST_Global* node) override {}

src/codegen/ast_interpreter.cpp

@@ -88,10 +88,8 @@ private:
     Value visit_assign(AST_Assign* node);
     Value visit_binop(AST_BinOp* node);
     Value visit_call(AST_Call* node);
-    Value visit_classDef(AST_ClassDef* node);
     Value visit_compare(AST_Compare* node);
     Value visit_delete(AST_Delete* node);
-    Value visit_functionDef(AST_FunctionDef* node);
     Value visit_global(AST_Global* node);
     Value visit_module(AST_Module* node);
     Value visit_print(AST_Print* node);
@@ -117,6 +115,8 @@ private:
     Value visit_tuple(AST_Tuple* node);
     Value visit_yield(AST_Yield* node);
 
+    Value visit_makeClass(AST_MakeClass* node);
+    Value visit_makeFunction(AST_MakeFunction* node);
 
     // pseudo
     Value visit_augBinOp(AST_AugBinOp* node);
@@ -606,14 +606,10 @@ Value ASTInterpreter::visit_stmt(AST_stmt* node) {
             return visit_assert((AST_Assert*)node);
         case AST_TYPE::Assign:
             return visit_assign((AST_Assign*)node);
-        case AST_TYPE::ClassDef:
-            return visit_classDef((AST_ClassDef*)node);
         case AST_TYPE::Delete:
             return visit_delete((AST_Delete*)node);
         case AST_TYPE::Expr:
             return visit_expr((AST_Expr*)node);
-        case AST_TYPE::FunctionDef:
-            return visit_functionDef((AST_FunctionDef*)node);
         case AST_TYPE::Pass:
             return Value(); // nothing todo
         case AST_TYPE::Print:
@@ -691,7 +687,8 @@ Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::v
     return boxCLFunction(cl, closure, is_generator, u.il);
 }
 
-Value ASTInterpreter::visit_functionDef(AST_FunctionDef* node) {
+Value ASTInterpreter::visit_makeFunction(AST_MakeFunction* mkfn) {
+    AST_FunctionDef* node = mkfn->function_def;
     AST_arguments* args = node->args;
 
     std::vector<Box*, StlCompatAllocator<Box*>> decorators;
@@ -703,11 +700,11 @@ Value ASTInterpreter::visit_functionDef(AST_FunctionDef* node) {
     for (int i = decorators.size() - 1; i >= 0; i--)
         func = runtimeCall(decorators[i], ArgPassSpec(1), func, 0, 0, 0, 0);
 
-    doStore(source_info->mangleName(node->name), func);
-    return Value();
+    return Value(func);
 }
 
-Value ASTInterpreter::visit_classDef(AST_ClassDef* node) {
+Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) {
+    AST_ClassDef* node = mkclass->class_def;
     ScopeInfo* scope_info = source_info->scoping->getScopeInfoForNode(node);
     assert(scope_info);
 
@@ -730,8 +727,7 @@ Value ASTInterpreter::visit_classDef(AST_ClassDef* node) {
     for (int i = decorators.size() - 1; i >= 0; i--)
         classobj = runtimeCall(decorators[i], ArgPassSpec(1), classobj, 0, 0, 0, 0);
 
-    doStore(source_info->mangleName(node->name), classobj);
-    return Value();
+    return Value(classobj);
 }
 
 Value ASTInterpreter::visit_raise(AST_Raise* node) {
@@ -897,6 +893,10 @@ Value ASTInterpreter::visit_expr(AST_expr* node) {
             return visit_clsAttribute((AST_ClsAttribute*)node);
         case AST_TYPE::LangPrimitive:
             return visit_langPrimitive((AST_LangPrimitive*)node);
+        case AST_TYPE::MakeClass:
+            return visit_makeClass((AST_MakeClass*)node);
+        case AST_TYPE::MakeFunction:
+            return visit_makeFunction((AST_MakeFunction*)node);
         default:
             RELEASE_ASSERT(0, "");
     };

src/core/ast.cpp

@@ -953,7 +953,6 @@ void AST_Branch::accept_stmt(StmtVisitor* v) {
     v->visit_branch(this);
 }
 
-
 void AST_Jump::accept(ASTVisitor* v) {
     bool skip = v->visit_jump(this);
     if (skip)
@@ -976,7 +975,29 @@ void* AST_ClsAttribute::accept_expr(ExprVisitor* v) {
     return v->visit_clsattribute(this);
 }
 
+void AST_MakeFunction::accept(ASTVisitor* v) {
+    bool skip = v->visit_makefunction(this);
+    if (skip)
+        return;
+
+    function_def->accept(v);
+}
+
+void* AST_MakeFunction::accept_expr(ExprVisitor* v) {
+    return v->visit_makefunction(this);
+}
 
+void AST_MakeClass::accept(ASTVisitor* v) {
+    bool skip = v->visit_makeclass(this);
+    if (skip)
+        return;
+
+    class_def->accept(v);
+}
+
+void* AST_MakeClass::accept_expr(ExprVisitor* v) {
+    return v->visit_makeclass(this);
+}
 
 void print_ast(AST* ast) {
     PrintVisitor v;
@@ -1851,6 +1872,16 @@ bool PrintVisitor::visit_clsattribute(AST_ClsAttribute* node) {
     return true;
 }
 
+bool PrintVisitor::visit_makefunction(AST_MakeFunction* node) {
+    printf("make_");
+    return false;
+}
+
+bool PrintVisitor::visit_makeclass(AST_MakeClass* node) {
+    printf("make_");
+    return false;
+}
+
 class FlattenVisitor : public ASTVisitor {
 private:
     std::vector<AST*>* output;
@@ -2098,6 +2129,15 @@ public:
         output->push_back(node);
         return false;
     }
+
+    virtual bool visit_makeclass(AST_MakeClass* node) {
+        output->push_back(node);
+        return false;
+    }
+    virtual bool visit_makefunction(AST_MakeFunction* node) {
+        output->push_back(node);
+        return false;
+    }
 };
 
 void flatten(const std::vector<AST_stmt*>& roots, std::vector<AST*>& output, bool expand_scopes) {

src/core/ast.h

@@ -128,6 +128,8 @@ enum AST_TYPE {
     AugBinOp = 203,
     Invoke = 204,
     LangPrimitive = 205,
+    MakeClass = 206,    // wraps a ClassDef to make it an expr
+    MakeFunction = 207, // wraps a FunctionDef to make it an expr
 
     // These aren't real AST types, but since we use AST types to represent binexp types
     // and divmod+truediv are essentially types of binops, we add them here (at least for now):
@@ -951,6 +953,31 @@ public:
     static const AST_TYPE::AST_TYPE TYPE = AST_TYPE::Yield;
 };
 
+class AST_MakeFunction : public AST_expr {
+public:
+    AST_FunctionDef* function_def;
+
+    virtual void accept(ASTVisitor* v);
+    virtual void* accept_expr(ExprVisitor* v);
+
+    AST_MakeFunction(AST_FunctionDef* fd)
+        : AST_expr(AST_TYPE::MakeFunction, fd->lineno, fd->col_offset), function_def(fd) {}
+
+    static const AST_TYPE::AST_TYPE TYPE = AST_TYPE::MakeFunction;
+};
+
+class AST_MakeClass : public AST_expr {
+public:
+    AST_ClassDef* class_def;
+
+    virtual void accept(ASTVisitor* v);
+    virtual void* accept_expr(ExprVisitor* v);
+
+    AST_MakeClass(AST_ClassDef* cd) : AST_expr(AST_TYPE::MakeClass, cd->lineno, cd->col_offset), class_def(cd) {}
+
+    static const AST_TYPE::AST_TYPE TYPE = AST_TYPE::MakeClass;
+};
+
 
 // AST pseudo-nodes that will get added during CFG-construction.  These don't exist in the input AST, but adding them in
 // lets us avoid creating a completely new IR for this phase
@@ -1116,6 +1143,8 @@ public:
     virtual bool visit_with(AST_With* node) { RELEASE_ASSERT(0, ""); }
     virtual bool visit_yield(AST_Yield* node) { RELEASE_ASSERT(0, ""); }
 
+    virtual bool visit_makeclass(AST_MakeClass* node) { RELEASE_ASSERT(0, ""); }
+    virtual bool visit_makefunction(AST_MakeFunction* node) { RELEASE_ASSERT(0, ""); }
     virtual bool visit_branch(AST_Branch* node) { RELEASE_ASSERT(0, ""); }
     virtual bool visit_jump(AST_Jump* node) { RELEASE_ASSERT(0, ""); }
 };
@@ -1188,6 +1217,8 @@ public:
 
     virtual bool visit_branch(AST_Branch* node) { return false; }
     virtual bool visit_jump(AST_Jump* node) { return false; }
+    virtual bool visit_makeclass(AST_MakeClass* node) { return false; }
+    virtual bool visit_makefunction(AST_MakeFunction* node) { return false; }
 };
 
 class ExprVisitor {
@@ -1224,6 +1255,8 @@ public:
     virtual void* visit_tuple(AST_Tuple* node) { RELEASE_ASSERT(0, ""); }
     virtual void* visit_unaryop(AST_UnaryOp* node) { RELEASE_ASSERT(0, ""); }
     virtual void* visit_yield(AST_Yield* node) { RELEASE_ASSERT(0, ""); }
+    virtual void* visit_makeclass(AST_MakeClass* node) { RELEASE_ASSERT(0, ""); }
+    virtual void* visit_makefunction(AST_MakeFunction* node) { RELEASE_ASSERT(0, ""); }
 };
 
 class StmtVisitor {
@@ -1333,6 +1366,8 @@ public:
 
     virtual bool visit_branch(AST_Branch* node);
     virtual bool visit_jump(AST_Jump* node);
+    virtual bool visit_makefunction(AST_MakeFunction* node);
+    virtual bool visit_makeclass(AST_MakeClass* node);
 };
 
 // Given an AST node, return a vector of the node plus all its descendents.

src/core/cfg.cpp

@@ -799,7 +799,7 @@ private:
     // Generates a FunctionDef which produces scope for `node'. The function produced is empty, so you'd better fill it.
     // `node' had better be a kind of node that scoping_analysis thinks can carry scope (see the switch (node->type)
     // block in ScopingAnalysis::processNameUsages in analysis/scoping_analysis.cpp); e.g. a Lambda or GeneratorExp.
-    AST_FunctionDef* makeFunctionForScope(AST* node) {
+    AST_MakeFunction* makeFunctionForScope(AST* node) {
         AST_FunctionDef* func = new AST_FunctionDef();
         func->lineno = node->lineno;
         func->col_offset = node->col_offset;
@@ -809,7 +809,7 @@ private:
         func->args->vararg = internString("");
         func->args->kwarg = internString("");
         scoping_analysis->registerScopeReplacement(node, func); // critical bit
-        return func;
+        return new AST_MakeFunction(func);
     }
 
     // This is a helper function used for generator expressions and comprehensions.
@@ -851,18 +851,18 @@ private:
         AST_expr* first = remapExpr(node->generators[0]->iter);
         InternedString first_generator_name = nodeName(node->generators[0]);
 
-        AST_FunctionDef* func = makeFunctionForScope(node);
-        func->args->args.push_back(makeName(first_generator_name, AST_TYPE::Param, node->lineno));
-        emitComprehensionLoops(&func->body, node->generators,
+        AST_MakeFunction* func = makeFunctionForScope(node);
+        func->function_def->args->args.push_back(makeName(first_generator_name, AST_TYPE::Param, node->lineno));
+        emitComprehensionLoops(&func->function_def->body, node->generators,
                                makeName(first_generator_name, AST_TYPE::Load, node->lineno),
                                [this, node](std::vector<AST_stmt*>* insert_point) {
                                    auto y = new AST_Yield();
                                    y->value = node->elt;
                                    insert_point->push_back(makeExpr(y));
                                });
-        push_back(func);
+        pushAssign(func->function_def->name, func);
 
-        return makeCall(makeName(func->name, AST_TYPE::Load, node->lineno), first);
+        return makeCall(makeName(func->function_def->name, AST_TYPE::Load, node->lineno), first);
     }
 
     void emitComprehensionYield(AST_DictComp* node, InternedString dict_name, std::vector<AST_stmt*>* insert_point) {
@@ -883,27 +883,27 @@ private:
         AST_expr* first = remapExpr(node->generators[0]->iter);
         InternedString first_generator_name = nodeName(node->generators[0]);
 
-        AST_FunctionDef* func = makeFunctionForScope(node);
-        func->args->args.push_back(makeName(first_generator_name, AST_TYPE::Param, node->lineno));
+        AST_MakeFunction* func = makeFunctionForScope(node);
+        func->function_def->args->args.push_back(makeName(first_generator_name, AST_TYPE::Param, node->lineno));
 
         InternedString rtn_name = nodeName(node);
         auto asgn = new AST_Assign();
         asgn->targets.push_back(makeName(rtn_name, AST_TYPE::Store, node->lineno));
         asgn->value = new ResultType();
-        func->body.push_back(asgn);
+        func->function_def->body.push_back(asgn);
 
         auto lambda =
             [&](std::vector<AST_stmt*>* insert_point) { emitComprehensionYield(node, rtn_name, insert_point); };
         AST_Name* first_name = makeName(first_generator_name, AST_TYPE::Load, node->lineno);
-        emitComprehensionLoops(&func->body, node->generators, first_name, lambda);
+        emitComprehensionLoops(&func->function_def->body, node->generators, first_name, lambda);
 
         auto rtn = new AST_Return();
         rtn->value = makeName(rtn_name, AST_TYPE::Load, node->lineno);
-        func->body.push_back(rtn);
+        func->function_def->body.push_back(rtn);
 
-        push_back(func);
+        pushAssign(func->function_def->name, func);
 
-        return makeCall(makeName(func->name, AST_TYPE::Load, node->lineno), first);
+        return makeCall(makeName(func->function_def->name, AST_TYPE::Load, node->lineno), first);
     }
 
     AST_expr* remapIfExp(AST_IfExp* node) {
@@ -957,14 +957,25 @@ private:
         return rtn;
     }
 
-    AST_expr* remapLambda(AST_Lambda* node) {
-        // Remap in place: see note in visit_functiondef for why.
-
-        for (int i = 0; i < node->args->defaults.size(); i++) {
-            node->args->defaults[i] = remapExpr(node->args->defaults[i]);
-        }
+    AST_arguments* remapArguments(AST_arguments* args) {
+        auto rtn = new AST_arguments();
+        rtn = new AST_arguments();
+        // don't remap args, they're not evaluated. NB. expensive vector copy.
+        rtn->args = args->args;
+        rtn->kwarg = args->kwarg;
+        rtn->vararg = args->vararg;
+        for (auto expr : args->defaults)
+            rtn->defaults.push_back(remapExpr(expr));
+        return rtn;
+    }
 
-        return node;
+    AST_expr* remapLambda(AST_Lambda* node) {
+        auto rtn = new AST_Lambda();
+        rtn->body = node->body; // don't remap now; will be CFG'ed later
+        rtn->args = remapArguments(node->args);
+        // lambdas create scope, need to register as replacement
+        scoping_analysis->registerScopeReplacement(node, rtn);
+        return rtn;
     }
 
     AST_expr* remapLangPrimitive(AST_LangPrimitive* node) {
@@ -1277,42 +1288,47 @@ public:
     }
 
     bool visit_classdef(AST_ClassDef* node) override {
-        // Remap in place: see note in visit_functiondef for why.
-
-        // Decorators are evaluated before the defaults:
-        for (int i = 0; i < node->decorator_list.size(); i++) {
-            node->decorator_list[i] = remapExpr(node->decorator_list[i]);
-        }
-
-        for (int i = 0; i < node->bases.size(); i++) {
-            node->bases[i] = remapExpr(node->bases[i]);
-        }
+        // waitaminute, who deallocates `node'?
+        auto def = new AST_ClassDef();
+        def->lineno = node->lineno;
+        def->col_offset = node->col_offset;
+        def->name = node->name;
+        def->body = node->body; // expensive vector copy
+
+        // Decorators are evaluated before bases:
+        for (auto expr : node->decorator_list)
+            def->decorator_list.push_back(remapExpr(expr));
+        for (auto expr : node->bases)
+            def->bases.push_back(remapExpr(expr));
+
+        scoping_analysis->registerScopeReplacement(node, def);
+
+        auto tmp = nodeName(node);
+        pushAssign(tmp, new AST_MakeClass(def));
+        // is this name mangling correct?
+        pushAssign(source->mangleName(def->name), makeName(tmp, AST_TYPE::Load, node->lineno));
 
-        push_back(node);
         return true;
     }
 
+    // FIXME: make this use MakeFunction
     bool visit_functiondef(AST_FunctionDef* node) override {
-        // As much as I don't like it, for now we're remapping these in place.
-        // This is because we do certain analyses pre-remapping, and associate the
-        // results with the node.  We can either do some refactoring and have a way
-        // of associating the new node with the same results, or just do the remapping
-        // in-place.
-        // Doing it in-place seems ugly, but I can't think of anything it should break,
-        // so just do that for now.
-        // TODO If we remap these (functiondefs, lambdas, classdefs) in place, we should probably
-        // remap everything in place?
+        auto def = new AST_FunctionDef();
+        def->name = node->name;
+        def->body = node->body; // expensive vector copy
+        // Decorators are evaluated before the defaults, so this *must* go before remapArguments().
+        // TODO(rntz): do we have a test for this
+        for (auto expr : node->decorator_list)
+            def->decorator_list.push_back(remapExpr(expr));
+        def->args = remapArguments(node->args);
+
+        scoping_analysis->registerScopeReplacement(node, def);
+
+        auto tmp = nodeName(node);
+        pushAssign(tmp, new AST_MakeFunction(def));
+        // is this name mangling correct?
+        pushAssign(source->mangleName(def->name), makeName(tmp, AST_TYPE::Load, node->lineno));
 
-        // Decorators are evaluated before the defaults:
-        for (int i = 0; i < node->decorator_list.size(); i++) {
-            node->decorator_list[i] = remapExpr(node->decorator_list[i]);
-        }
-
-        for (int i = 0; i < node->args->defaults.size(); i++) {
-            node->args->defaults[i] = remapExpr(node->args->defaults[i]);
-        }
-
-        push_back(node);
         return true;
     }