BST: merge MakeFunction with FunctionDef and MakeClass with ClassDef

this simplifies the code and removes a lot of ugliness

src/analysis/type_analysis.cpp

@@ -481,9 +481,7 @@ private:
         }
     }
 
-    void visit_makeclass(BST_MakeClass* mkclass) override {
-        auto* node = bst_cast<BST_ClassDef>(getCodeConstants().getFuncOrClass(mkclass->index_class_def).first);
-
+    void visit_makeclass(BST_MakeClass* node) override {
         for (int i = 0; i < node->num_decorator; ++i) {
             getType(node->decorator[i]);
         }
@@ -492,7 +490,7 @@ private:
 
         // TODO should we speculate that classdefs will generally return a class?
         // return typeFromClass(type_cls);
-        _doSet(mkclass->vreg_dst, UNKNOWN);
+        _doSet(node->vreg_dst, UNKNOWN);
     }
 
     void visit_deletesub(BST_DeleteSub* node) override { getType(node->vreg_value); }
@@ -507,9 +505,7 @@ private:
             assert(node->vreg == VREG_UNDEFINED);
     }
 
-    void visit_makefunction(BST_MakeFunction* mkfn) override {
-        auto* node = bst_cast<BST_FunctionDef>(getCodeConstants().getFuncOrClass(mkfn->index_func_def).first);
-
+    void visit_makefunction(BST_MakeFunction* node) override {
         for (int i = 0; i < node->num_defaults + node->num_decorator; ++i) {
             getType(node->elts[i]);
         }
@@ -517,7 +513,7 @@ private:
         CompilerType* t = UNKNOWN;
         if (node->num_decorator == 0)
             t = typeFromClass(function_cls);
-        _doSet(mkfn->vreg_dst, t);
+        _doSet(node->vreg_dst, t);
     }
 
     void visit_exec(BST_Exec* node) override {

src/codegen/ast_interpreter.cpp

@@ -76,7 +76,7 @@ public:
 private:
     Value executeStmt(BST_stmt* node);
 
-    Value createFunction(BST_FunctionDef* node, BoxedCode* node_code);
+    Value createFunction(BST_MakeFunction* node, BoxedCode* node_code);
     Value doBinOp(BST_stmt* node, Value left, Value right, int op, BinExpType exp_type);
     void doStore(int vreg, STOLEN(Value) value);
     void doStoreArg(BST_Name* name, STOLEN(Value) value);
@@ -1126,7 +1126,7 @@ Value ASTInterpreter::visit_return(BST_Return* node) {
     return s;
 }
 
-Value ASTInterpreter::createFunction(BST_FunctionDef* node, BoxedCode* code) {
+Value ASTInterpreter::createFunction(BST_MakeFunction* node, BoxedCode* code) {
     assert(code);
 
     std::vector<Box*> defaults;
@@ -1205,16 +1205,17 @@ Value ASTInterpreter::createFunction(BST_FunctionDef* node, BoxedCode* code) {
     return rtn;
 }
 
-Value ASTInterpreter::visit_makeFunction(BST_MakeFunction* mkfn) {
-    auto func_entry = getCodeConstants().getFuncOrClass(mkfn->index_func_def);
-    auto node = bst_cast<BST_FunctionDef>(func_entry.first);
+Value ASTInterpreter::visit_makeFunction(BST_MakeFunction* node) {
+    BoxedCode* code = (BoxedCode*)getCodeConstants().getConstant(node->vreg_code_obj);
+    assert(code);
+    assert(code->cls == code_cls);
 
     std::vector<Value> decorators;
     decorators.reserve(node->num_decorator);
     for (int i = 0; i < node->num_decorator; ++i)
         decorators.push_back(getVReg(node->elts[i]));
 
-    Value func = createFunction(node, func_entry.second);
+    Value func = createFunction(node, code);
 
     for (int i = decorators.size() - 1; i >= 0; i--) {
         func.o = runtimeCall(autoDecref(decorators[i].o), ArgPassSpec(1), autoDecref(func.o), 0, 0, 0, 0);
@@ -1227,12 +1228,9 @@ Value ASTInterpreter::visit_makeFunction(BST_MakeFunction* mkfn) {
     return func;
 }
 
-Value ASTInterpreter::visit_makeClass(BST_MakeClass* mkclass) {
+Value ASTInterpreter::visit_makeClass(BST_MakeClass* node) {
     abortJITing();
 
-    auto class_entry = getCodeConstants().getFuncOrClass(mkclass->index_class_def);
-    auto node = bst_cast<BST_ClassDef>(class_entry.first);
-
     BoxedTuple* bases_tuple = (BoxedTuple*)getVReg(node->vreg_bases_tuple).o;
     assert(bases_tuple->cls == tuple_cls);
     AUTO_DECREF(bases_tuple);
@@ -1243,8 +1241,9 @@ Value ASTInterpreter::visit_makeClass(BST_MakeClass* mkclass) {
         decorators.push_back(getVReg(node->decorator[i]).o);
     }
 
-    BoxedCode* code = class_entry.second;
+    BoxedCode* code = (BoxedCode*)getCodeConstants().getConstant(node->vreg_code_obj);
     assert(code);
+    assert(code->cls == code_cls);
 
     const ScopingResults& scope_info = code->source->scoping;
 

src/codegen/irgen/irgenerator.cpp

@@ -1488,10 +1488,7 @@ private:
         return new ConcreteCompilerVariable(UNKNOWN, rtn);
     }
 
-    CompilerVariable* evalMakeClass(BST_MakeClass* mkclass, const UnwindInfo& unw_info) {
-        auto class_entry = irstate->getCodeConstants().getFuncOrClass(mkclass->index_class_def);
-        BST_ClassDef* node = bst_cast<BST_ClassDef>(class_entry.first);
-
+    CompilerVariable* evalMakeClass(BST_MakeClass* node, const UnwindInfo& unw_info) {
         CompilerVariable* _bases_tuple = evalVReg(node->vreg_bases_tuple);
         ConcreteCompilerVariable* bases_tuple = _bases_tuple->makeConverted(emitter, _bases_tuple->getBoxType());
 
@@ -1500,8 +1497,9 @@ private:
             decorators.push_back(evalVReg(node->decorator[i]));
         }
 
-        BoxedCode* code = class_entry.second;
+        BoxedCode* code = (BoxedCode*)irstate->getCodeConstants().getConstant(node->vreg_code_obj);
         assert(code);
+        assert(code->cls == code_cls);
         const ScopingResults& scope_info = code->source->scoping;
 
         // TODO duplication with _createFunction:
@@ -1544,7 +1542,7 @@ private:
         return cls;
     }
 
-    CompilerVariable* _createFunction(BST_FunctionDef* node, BoxedCode* code, const UnwindInfo& unw_info) {
+    CompilerVariable* _createFunction(BST_MakeFunction* node, BoxedCode* code, const UnwindInfo& unw_info) {
         assert(code);
 
         std::vector<ConcreteCompilerVariable*> defaults;
@@ -1572,16 +1570,17 @@ private:
         return func;
     }
 
-    CompilerVariable* evalMakeFunction(BST_MakeFunction* mkfn, const UnwindInfo& unw_info) {
-        auto func_entry = irstate->getCodeConstants().getFuncOrClass(mkfn->index_func_def);
-        BST_FunctionDef* node = bst_cast<BST_FunctionDef>(func_entry.first);
+    CompilerVariable* evalMakeFunction(BST_MakeFunction* node, const UnwindInfo& unw_info) {
+        BoxedCode* code = (BoxedCode*)irstate->getCodeConstants().getConstant(node->vreg_code_obj);
+        assert(code);
+        assert(code->cls == code_cls);
 
         std::vector<CompilerVariable*> decorators;
         for (int i = 0; i < node->num_decorator; ++i) {
             decorators.push_back(evalVReg(node->elts[i]));
         }
 
-        CompilerVariable* func = _createFunction(node, func_entry.second, unw_info);
+        CompilerVariable* func = _createFunction(node, code, unw_info);
 
         for (int i = decorators.size() - 1; i >= 0; i--) {
             func = decorators[i]->call(emitter, getOpInfoForNode(node, unw_info), ArgPassSpec(1), { func }, NULL);

src/core/bst.cpp

@@ -183,23 +183,6 @@ void BST_Compare::accept_stmt(StmtVisitor* v) {
     return v->visit_compare(this);
 }
 
-void BST_ClassDef::accept(BSTVisitor* v) {
-    bool skip = v->visit_classdef(this);
-    if (skip)
-        return;
-
-    v->visit_vreg(&vreg_bases_tuple);
-    for (int i = 0; i < num_decorator; ++i) {
-        v->visit_vreg(&decorator[i]);
-    }
-
-    // we dont't visit the body
-}
-
-void BST_ClassDef::accept_stmt(StmtVisitor* v) {
-    v->visit_classdef(this);
-}
-
 void BST_DeleteAttr::accept(BSTVisitor* v) {
     bool skip = v->visit_deleteattr(this);
     if (skip)
@@ -276,21 +259,6 @@ void BST_Exec::accept_stmt(StmtVisitor* v) {
     v->visit_exec(this);
 }
 
-void BST_FunctionDef::accept(BSTVisitor* v) {
-    bool skip = v->visit_functiondef(this);
-    if (skip)
-        return;
-
-    for (int i = 0; i < num_decorator + num_defaults; ++i) {
-        v->visit_vreg(&elts[i]);
-    }
-    // we dont't visit the body
-}
-
-void BST_FunctionDef::accept_stmt(StmtVisitor* v) {
-    v->visit_functiondef(this);
-}
-
 void BST_Landingpad::accept(BSTVisitor* v) {
     bool skip = v->visit_landingpad(this);
     if (skip)
@@ -717,7 +685,9 @@ void BST_MakeFunction::accept(BSTVisitor* v) {
     if (skip)
         return;
 
-    bst_cast<BST_FunctionDef>(v->getCodeConstants().getFuncOrClass(index_func_def).first)->accept(v);
+    for (int i = 0; i < num_decorator + num_defaults; ++i) {
+        v->visit_vreg(&elts[i]);
+    }
     v->visit_vreg(&vreg_dst, true);
 }
 
@@ -730,7 +700,10 @@ void BST_MakeClass::accept(BSTVisitor* v) {
     if (skip)
         return;
 
-    bst_cast<BST_ClassDef>(v->getCodeConstants().getFuncOrClass(index_class_def).first)->accept(v);
+    v->visit_vreg(&vreg_bases_tuple);
+    for (int i = 0; i < num_decorator; ++i) {
+        v->visit_vreg(&decorator[i]);
+    }
     v->visit_vreg(&vreg_dst, true);
 }
 
@@ -931,35 +904,6 @@ bool PrintVisitor::visit_compare(BST_Compare* node) {
     return true;
 }
 
-bool PrintVisitor::visit_classdef(BST_ClassDef* node) {
-    check_if_invoke(node);
-
-    for (int i = 0, n = node->num_decorator; i < n; i++) {
-        stream << "@";
-        visit_vreg(&node->decorator[i]);
-        stream << "\n";
-        printIndent();
-    }
-    stream << "class " << code_constants.getInternedString(node->index_name).s() << "(";
-    visit_vreg(&node->vreg_bases_tuple);
-    stream << ")";
-
-    indent += 4;
-    stream << '\n';
-    printIndent();
-    stream << "...";
-#if 0
-    for (int i = 0, n = node->body.size(); i < n; i++) {
-        stream << "\n";
-        printIndent();
-        node->body[i]->accept(this);
-    }
-#endif
-    indent -= 4;
-
-    return true;
-}
-
 bool PrintVisitor::visit_deletesub(BST_DeleteSub* node) {
     check_if_invoke(node);
 
@@ -1033,48 +977,6 @@ bool PrintVisitor::visit_exec(BST_Exec* node) {
     return true;
 }
 
-bool PrintVisitor::visit_functiondef(BST_FunctionDef* node) {
-    check_if_invoke(node);
-
-    for (int i = 0; i < node->num_decorator; ++i) {
-        stream << "@";
-        visit_vreg(&node->elts[i]);
-        stream << "\n";
-        printIndent();
-    }
-
-    stream << "def ";
-    if (node->index_name != VREG_UNDEFINED)
-        stream << code_constants.getInternedString(node->index_name).s();
-    else
-        stream << "<lambda>";
-    stream << "(";
-
-    for (int i = 0; i < node->num_defaults; ++i) {
-        if (i > 0)
-            stream << ", ";
-
-        stream << "<default " << i << ">=";
-        visit_vreg(&node->elts[node->num_decorator + i]);
-    }
-
-    stream << ")";
-
-    indent += 4;
-    stream << '\n';
-    printIndent();
-    stream << "...";
-#if 0
-    for (int i = 0; i < node->body.size(); i++) {
-        stream << "\n";
-        printIndent();
-        node->body[i]->accept(this);
-    }
-#endif
-    indent -= 4;
-    return true;
-}
-
 bool PrintVisitor::visit_landingpad(BST_Landingpad* node) {
     check_if_invoke(node);
 
@@ -1483,15 +1385,64 @@ bool PrintVisitor::visit_makefunction(BST_MakeFunction* node) {
     check_if_invoke(node);
 
     visit_vreg(&node->vreg_dst, true);
-    stream << "make_";
-    return false;
+    stream << "make_def ";
+
+    for (int i = 0; i < node->num_decorator; ++i) {
+        stream << "@";
+        visit_vreg(&node->elts[i]);
+        stream << "\n";
+        printIndent();
+    }
+
+    if (node->index_name != VREG_UNDEFINED)
+        stream << code_constants.getInternedString(node->index_name).s();
+    else
+        stream << "<lambda>";
+    stream << "(";
+
+    for (int i = 0; i < node->num_defaults; ++i) {
+        if (i > 0)
+            stream << ", ";
+
+        stream << "<default " << i << ">=";
+        visit_vreg(&node->elts[node->num_decorator + i]);
+    }
+
+    stream << ")\n";
+
+    indent += 4;
+    printIndent();
+    visit_vreg(&node->vreg_code_obj);
+    stream << "\n";
+    printIndent();
+    stream << "...\n";
+    indent -= 4;
+    return true;
 }
 
 bool PrintVisitor::visit_makeclass(BST_MakeClass* node) {
     check_if_invoke(node);
 
     visit_vreg(&node->vreg_dst, true);
-    stream << "make_";
-    return false;
+    stream << "make_class ";
+
+    for (int i = 0, n = node->num_decorator; i < n; i++) {
+        stream << "@";
+        visit_vreg(&node->decorator[i]);
+        stream << "\n";
+        printIndent();
+    }
+    stream << code_constants.getInternedString(node->index_name).s() << "(";
+    visit_vreg(&node->vreg_bases_tuple);
+    stream << ")\n";
+
+    indent += 4;
+    printIndent();
+    visit_vreg(&node->vreg_code_obj);
+    stream << "\n";
+    printIndent();
+    stream << "...\n";
+    indent -= 4;
+    return true;
 }
 }

src/core/cfg.cpp

@@ -1464,8 +1464,10 @@ private:
         static BoxedString* gen_name = getStaticString("<generator>");
 
         BoxedCode* code = cfgizer->runRecursively(new_body, gen_name, node->lineno, genexp_args, node);
-        BST_FunctionDef* func = BST_FunctionDef::create(0, 0);
-        BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(func, code_constants.addFuncOrClass(func, code));
+        BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(0, 0);
+        mkfunc->lineno = node->lineno;
+        mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
+        code_constants.addOwnedRef(code);
         TmpValue func_var_name = createDstName(mkfunc);
 
         return makeCall(func_var_name, { first });
@@ -1522,8 +1524,10 @@ private:
         static BoxedString* comp_name = getStaticString("<comperehension>");
 
         BoxedCode* code = cfgizer->runRecursively(new_body, comp_name, node->lineno, args, node);
-        BST_FunctionDef* func = BST_FunctionDef::create(0, 0);
-        BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(func, code_constants.addFuncOrClass(func, code));
+        BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(0, 0);
+        mkfunc->lineno = node->lineno;
+        mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
+        code_constants.addOwnedRef(code);
         TmpValue func_var_name = createDstName(mkfunc);
 
         return makeCall(func_var_name, { first });
@@ -1564,16 +1568,22 @@ private:
 
         stmt->value = node->body; // don't remap now; will be CFG'ed later
 
-        auto bdef = BST_FunctionDef::create(0 /* decorators */, node->args->defaults.size());
-        bdef->lineno = node->lineno;
+        llvm::SmallVector<TmpValue, 8> remapped_defaults;
+        for (int i = 0; i < node->args->defaults.size(); ++i) {
+            remapped_defaults.emplace_back(remapExpr(node->args->defaults[i]));
+        }
+
+        auto mkfn = allocAndPush<BST_MakeFunction>(0 /* decorators */, node->args->defaults.size());
+        mkfn->lineno = node->lineno;
 
         for (int i = 0; i < node->args->defaults.size(); ++i) {
-            unmapExpr(remapExpr(node->args->defaults[i]), &bdef->elts[i]);
+            unmapExpr(remapped_defaults[i], &mkfn->elts[i]);
         }
 
         auto name = getStaticString("<lambda>");
-        auto* code = cfgizer->runRecursively({ stmt }, name, node->lineno, node->args, node);
-        auto mkfn = allocAndPush<BST_MakeFunction>(bdef, code_constants.addFuncOrClass(bdef, code));
+        auto* code = cfgizer->runRecursively({ stmt }, name, mkfn->lineno, node->args, node);
+        mkfn->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
+        code_constants.addOwnedRef(code);
 
         return createDstName(mkfn);
     }
@@ -1896,18 +1906,20 @@ public:
         }
         TmpValue bases_name = createDstName(bases);
 
-        auto def = BST_ClassDef::create(node->decorator_list.size());
-        def->lineno = node->lineno;
-        def->index_name = remapInternedString(node->name);
+        auto mkclass = allocAndPush<BST_MakeClass>(node->decorator_list.size());
+        mkclass->lineno = node->lineno;
+        mkclass->index_name = remapInternedString(node->name);
 
         for (int i = 0; i < node->decorator_list.size(); ++i) {
-            unmapExpr(remapped_deco[i], &def->decorator[i]);
+            unmapExpr(remapped_deco[i], &mkclass->decorator[i]);
         }
 
-        unmapExpr(bases_name, &def->vreg_bases_tuple);
+        unmapExpr(bases_name, &mkclass->vreg_bases_tuple);
 
         auto* code = cfgizer->runRecursively(node->body, node->name.getBox(), node->lineno, NULL, node);
-        auto mkclass = allocAndPush<BST_MakeClass>(def, code_constants.addFuncOrClass(def, code));
+        mkclass->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
+        code_constants.addOwnedRef(code);
+
         auto tmp = createDstName(mkclass);
         pushAssign(TmpValue(scoping->mangleName(node->name), node->lineno), tmp);
 
@@ -1926,21 +1938,22 @@ public:
             remapped_defaults.emplace_back(remapExpr(node->args->defaults[i]));
         }
 
-        auto def = BST_FunctionDef::create(node->decorator_list.size(), node->args->defaults.size());
-        def->lineno = node->lineno;
-        def->index_name = remapInternedString(node->name);
+        auto mkfunc = allocAndPush<BST_MakeFunction>(node->decorator_list.size(), node->args->defaults.size());
+        mkfunc->lineno = node->lineno;
+        mkfunc->index_name = remapInternedString(node->name);
 
         // Decorators are evaluated before the defaults, so this *must* go before remapArguments().
         // TODO(rntz): do we have a test for this
         for (int i = 0; i < node->decorator_list.size(); ++i) {
-            unmapExpr(remapped_deco[i], &def->elts[i]);
+            unmapExpr(remapped_deco[i], &mkfunc->elts[i]);
         }
         for (int i = 0; i < node->args->defaults.size(); ++i) {
-            unmapExpr(remapped_defaults[i], &def->elts[node->decorator_list.size() + i]);
+            unmapExpr(remapped_defaults[i], &mkfunc->elts[node->decorator_list.size() + i]);
         }
 
         auto* code = cfgizer->runRecursively(node->body, node->name.getBox(), node->lineno, node->args, node);
-        auto mkfunc = allocAndPush<BST_MakeFunction>(def, code_constants.addFuncOrClass(def, code));
+        mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
+        code_constants.addOwnedRef(code);
         auto tmp = createDstName(mkfunc);
         pushAssign(TmpValue(scoping->mangleName(node->name), node->lineno), tmp);
 

src/runtime/types.cpp

@@ -4053,12 +4053,6 @@ BORROWED(BoxedFloat*) CodeConstants::getFloatConstant(double d) const {
 void CodeConstants::dealloc() const {
     decrefArray(owned_refs.data(), owned_refs.size());
     owned_refs.clear();
-    for (auto&& e : funcs_and_classes) {
-        Py_DECREF(e.second);
-        assert(e.first->type() == BST_TYPE::FunctionDef || e.first->type() == BST_TYPE::ClassDef);
-        delete[] e.first;
-    }
-    funcs_and_classes.clear();
 }
 
 #ifndef Py_REF_DEBUG

src/runtime/types.h

@@ -1079,8 +1079,6 @@ private:
     // all objects we need to decref when the code object dies
     mutable std::vector<Box*> owned_refs;
 
-    mutable std::vector<std::pair<BST_stmt*, BoxedCode*>> funcs_and_classes;
-
     // Note: DenseMap doesn't work here since we don't prevent the tombstone/empty
     // keys from reaching it.
     mutable std::unordered_map<int64_t, BoxedInt*> int_constants;
@@ -1113,17 +1111,8 @@ public:
     BORROWED(BoxedInt*) getIntConstant(int64_t n) const;
     BORROWED(BoxedFloat*) getFloatConstant(double d) const;
 
-    std::pair<BST_stmt*, BORROWED(BoxedCode*)> getFuncOrClass(int constant) const {
-        return funcs_and_classes[constant];
-    }
-
     int addInternedString(InternedString s) { return createVRegEntryForConstant(s.getBox()); }
 
-    int addFuncOrClass(BST_stmt* stmt, STOLEN(BoxedCode*) code) {
-        funcs_and_classes.emplace_back(stmt, code);
-        return funcs_and_classes.size() - 1;
-    }
-
     int addKeywordNames(llvm::ArrayRef<BoxedString*> name) {
         keyword_names.emplace_back(new std::vector<BoxedString*>(name.begin(), name.end()));
         return keyword_names.size() - 1;

test/unittests/analysis.cpp

@@ -30,10 +30,11 @@ static BoxedCode* getCodeObjectOfFirstMakeFunction(BoxedCode* module_code) {
     for (BST_stmt* stmt : *module_code->source->cfg->getStartingBlock()) {
         if (stmt->type() !=  BST_TYPE::MakeFunction)
             continue;
-        code = module_code->code_constants.getFuncOrClass(bst_cast<BST_MakeFunction>(stmt)->index_func_def).second;
+        code = (BoxedCode*)module_code->code_constants.getConstant(bst_cast<BST_MakeFunction>(stmt)->vreg_code_obj);
+        assert(code);
+        assert(code->cls == code_cls);
         break;
     }
-    assert(code);
     return code;
 }