Merge pull request #1325 from undingen/remove_body

SourceInfo: remove the AST stmt copy, remove special handling of lambdas

src/analysis/function_analysis.cpp

@@ -98,12 +98,6 @@ public:
         return true;
     }
 
-    bool visit_lambda(AST_Lambda* node) {
-        for (auto* d : node->args->defaults)
-            d->accept(this);
-        return true;
-    }
-
     bool visit_name(AST_Name* node) {
         if (node->vreg == -1)
             return true;

src/analysis/type_analysis.cpp

@@ -391,8 +391,6 @@ private:
 
     void* visit_index(AST_Index* node) override { return getType(node->value); }
 
-    void* visit_lambda(AST_Lambda* node) override { return typeFromClass(function_cls); }
-
     void* visit_langprimitive(AST_LangPrimitive* node) override {
         switch (node->opcode) {
             case AST_LangPrimitive::CHECK_EXC_MATCH:

src/codegen/ast_interpreter.cpp

@@ -103,7 +103,6 @@ private:
     Value visit_expr(AST_Expr* node);
     Value visit_extslice(AST_ExtSlice* node);
     Value visit_index(AST_Index* node);
-    Value visit_lambda(AST_Lambda* node);
     Value visit_list(AST_List* node);
     Value visit_name(AST_Name* node);
     Value visit_num(AST_Num* node);
@@ -1481,8 +1480,6 @@ Value ASTInterpreter::visit_expr(AST_expr* node) {
             return visit_compare((AST_Compare*)node);
         case AST_TYPE::Dict:
             return visit_dict((AST_Dict*)node);
-        case AST_TYPE::Lambda:
-            return visit_lambda((AST_Lambda*)node);
         case AST_TYPE::List:
             return visit_list((AST_List*)node);
         case AST_TYPE::Name:
@@ -1640,15 +1637,6 @@ Value ASTInterpreter::visit_repr(AST_Repr* node) {
     return Value(repr(v.o), jit ? jit->emitRepr(v) : NULL);
 }
 
-Value ASTInterpreter::visit_lambda(AST_Lambda* node) {
-    AST_Return* expr = new AST_Return();
-    expr->value = node->body;
-    expr->lineno = node->body->lineno;
-
-    std::vector<AST_stmt*> body = { expr };
-    return createFunction(node, node->args, body);
-}
-
 Value ASTInterpreter::visit_dict(AST_Dict* node) {
     RELEASE_ASSERT(node->keys.size() == node->values.size(), "not implemented");
 
@@ -2053,7 +2041,7 @@ Box* astInterpretFunction(FunctionMetadata* md, Box* closure, Box* generator, Bo
     // (For instance, throwing the exception will try to fetch the current statement, but we determine
     // that by looking at the cfg.)
     if (!source_info->cfg)
-        source_info->cfg = computeCFG(source_info, source_info->body, md->param_names);
+        source_info->cfg = computeCFG(source_info, md->param_names);
 
     Box** vregs = NULL;
     int num_vregs = source_info->cfg->getVRegInfo().getTotalNumOfVRegs();
@@ -2095,7 +2083,7 @@ Box* astInterpretFunctionEval(FunctionMetadata* md, Box* globals, Box* boxedLoca
     // that by looking at the cfg.)
     SourceInfo* source_info = md->source.get();
     if (!source_info->cfg)
-        source_info->cfg = computeCFG(source_info, source_info->body, md->param_names);
+        source_info->cfg = computeCFG(source_info, md->param_names);
 
     Box** vregs = NULL;
     int num_vregs = source_info->cfg->getVRegInfo().getTotalNumOfVRegs();

src/codegen/codegen.cpp

@@ -93,15 +93,8 @@ void FunctionMetadata::addVersion(CompiledFunction* compiled) {
     }
 }
 
-SourceInfo::SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast,
-                       std::vector<AST_stmt*> body, BoxedString* fn)
-    : parent_module(m),
-      scoping(scoping),
-      scope_info(NULL),
-      future_flags(future_flags),
-      ast(ast),
-      cfg(NULL),
-      body(std::move(body)) {
+SourceInfo::SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast, BoxedString* fn)
+    : parent_module(m), scoping(scoping), scope_info(NULL), future_flags(future_flags), ast(ast), cfg(NULL) {
     assert(fn);
 
     // TODO: this is a very bad way of handling this:

src/codegen/cpython_ast.cpp

@@ -740,7 +740,15 @@ public:
             case Expression_kind: {
                 AST_Expression* rtn = new AST_Expression(llvm::make_unique<InternedStringPool>());
                 this->pool = rtn->interned_strings.get();
-                rtn->body = this->convert(mod->v.Expression.body);
+
+                // instead of storing the expression inside the AST node we convert it directly to a return statement
+                AST_expr* expr = this->convert(mod->v.Expression.body);
+                auto rtn_stmt = new AST_Return;
+                rtn_stmt->lineno = expr->lineno;
+                rtn_stmt->col_offset = expr->col_offset;
+                rtn_stmt->value = expr;
+
+                rtn->body = rtn_stmt;
                 return rtn;
             }
             default:

src/codegen/irgen/future.cpp

@@ -67,7 +67,7 @@ inline bool is_stmt_string(AST_stmt* stmt) {
     return stmt->type == AST_TYPE::Expr && static_cast<AST_Expr*>(stmt)->value->type == AST_TYPE::Str;
 }
 
-FutureFlags getFutureFlags(std::vector<AST_stmt*> const& body, const char* file) {
+FutureFlags getFutureFlags(llvm::ArrayRef<AST_stmt*> body, const char* file) {
     FutureFlags ff = 0;
 
     // Set the defaults for the future flags depending on what version we are

src/codegen/irgen/future.h

@@ -15,7 +15,7 @@
 #ifndef PYSTON_CODEGEN_IRGEN_FUTURE_H
 #define PYSTON_CODEGEN_IRGEN_FUTURE_H
 
-#include <vector>
+#include "llvm/ADT/ArrayRef.h"
 
 #include "core/types.h"
 
@@ -25,7 +25,7 @@ namespace pyston {
 // bad __future__ imports. Returns the futures that are turned on. This is used
 // for irgeneration; the parser still has to handle some futures on its own,
 // when they are relevant for the parser.
-FutureFlags getFutureFlags(std::vector<AST_stmt*> const& body, const char* file);
+FutureFlags getFutureFlags(llvm::ArrayRef<AST_stmt*> body, const char* file);
 }
 
 #endif

src/codegen/irgen/hooks.cpp

@@ -103,6 +103,21 @@ InternedString SourceInfo::mangleName(InternedString id) {
     return getScopeInfo()->mangleName(id);
 }
 
+llvm::ArrayRef<AST_stmt*> SourceInfo::getBody() const {
+    switch (ast->type) {
+        case AST_TYPE::ClassDef:
+            return ((AST_ClassDef*)ast)->body;
+        case AST_TYPE::Expression:
+            return ((AST_Expression*)ast)->body;
+        case AST_TYPE::FunctionDef:
+            return ((AST_FunctionDef*)ast)->body;
+        case AST_TYPE::Module:
+            return ((AST_Module*)ast)->body;
+        default:
+            RELEASE_ASSERT(0, "unknown %d", ast->type);
+    };
+}
+
 InternedStringPool& SourceInfo::getInternedStrings() {
     return scoping->getInternedStrings();
 }
@@ -122,8 +137,8 @@ BORROWED(BoxedString*) SourceInfo::getName() noexcept {
         case AST_TYPE::ClassDef:
             return ast_cast<AST_ClassDef>(ast)->name.getBox();
         case AST_TYPE::FunctionDef:
-            return ast_cast<AST_FunctionDef>(ast)->name.getBox();
-        case AST_TYPE::Lambda:
+            if (ast_cast<AST_FunctionDef>(ast)->name != InternedString())
+                return ast_cast<AST_FunctionDef>(ast)->name.getBox();
             return lambda_name;
         case AST_TYPE::Module:
         case AST_TYPE::Expression:
@@ -135,8 +150,7 @@ BORROWED(BoxedString*) SourceInfo::getName() noexcept {
 }
 
 Box* SourceInfo::getDocString() {
-    AST_Str* first_str = NULL;
-
+    auto body = getBody();
     if (body.size() > 0 && body[0]->type == AST_TYPE::Expr
         && static_cast<AST_Expr*>(body[0])->value->type == AST_TYPE::Str) {
         return boxString(static_cast<AST_Str*>(static_cast<AST_Expr*>(body[0])->value)->str_data);
@@ -275,7 +289,7 @@ CompiledFunction* compileFunction(FunctionMetadata* f, FunctionSpecialization* s
 
     // Do the analysis now if we had deferred it earlier:
     if (source->cfg == NULL) {
-        source->cfg = computeCFG(source, source->body, f->param_names);
+        source->cfg = computeCFG(source, f->param_names);
     }
 
 
@@ -336,7 +350,7 @@ void compileAndRunModule(AST_Module* m, BoxedModule* bm) {
 
         auto fn_str = boxString(fn);
         AUTO_DECREF(fn_str);
-        std::unique_ptr<SourceInfo> si(new SourceInfo(bm, scoping, future_flags, m, m->body, fn_str));
+        std::unique_ptr<SourceInfo> si(new SourceInfo(bm, scoping, future_flags, m, fn_str));
 
         static BoxedString* doc_str = getStaticString("__doc__");
         bm->setattr(doc_str, autoDecref(si->getDocString()), NULL);
@@ -375,7 +389,7 @@ Box* evalOrExec(FunctionMetadata* md, Box* globals, Box* boxedLocals) {
     return astInterpretFunctionEval(md, globals, boxedLocals);
 }
 
-static FunctionMetadata* compileForEvalOrExec(AST* source, std::vector<AST_stmt*> body, BoxedString* fn,
+static FunctionMetadata* compileForEvalOrExec(AST* source, llvm::ArrayRef<AST_stmt*> body, BoxedString* fn,
                                               PyCompilerFlags* flags) {
     LOCK_REGION(codegen_rwlock.asWrite());
 
@@ -394,8 +408,7 @@ static FunctionMetadata* compileForEvalOrExec(AST* source, std::vector<AST_stmt*
         flags->cf_flags = future_flags;
     }
 
-    std::unique_ptr<SourceInfo> si(
-        new SourceInfo(getCurrentModule(), scoping, future_flags, source, std::move(body), fn));
+    std::unique_ptr<SourceInfo> si(new SourceInfo(getCurrentModule(), scoping, future_flags, source, fn));
 
     FunctionMetadata* md = new FunctionMetadata(0, false, false, std::move(si));
     return md;
@@ -406,14 +419,7 @@ static FunctionMetadata* compileExec(AST_Module* parsedModule, BoxedString* fn,
 }
 
 static FunctionMetadata* compileEval(AST_Expression* parsedExpr, BoxedString* fn, PyCompilerFlags* flags) {
-    // We need body (list of statements) to compile.
-    // Obtain this by simply making a single statement which contains the expression.
-    AST_Return* stmt = new AST_Return();
-    stmt->lineno = parsedExpr->body->lineno;
-    stmt->value = parsedExpr->body;
-    std::vector<AST_stmt*> body = { stmt };
-
-    return compileForEvalOrExec(parsedExpr, std::move(body), fn, flags);
+    return compileForEvalOrExec(parsedExpr, parsedExpr->body, fn, flags);
 }
 
 extern "C" PyCodeObject* PyAST_Compile(struct _mod* _mod, const char* filename, PyCompilerFlags* flags,

src/codegen/irgen/irgenerator.cpp

@@ -1281,18 +1281,6 @@ private:
 
     CompilerVariable* evalIndex(AST_Index* node, const UnwindInfo& unw_info) { return evalExpr(node->value, unw_info); }
 
-    CompilerVariable* evalLambda(AST_Lambda* node, const UnwindInfo& unw_info) {
-        AST_Return* expr = new AST_Return();
-        expr->value = node->body;
-        expr->lineno = node->body->lineno;
-
-        std::vector<AST_stmt*> body = { expr };
-        CompilerVariable* func = _createFunction(node, unw_info, node->args, body);
-        ConcreteCompilerVariable* converted = func->makeConverted(emitter, func->getBoxType());
-
-        return converted;
-    }
-
 
     CompilerVariable* evalList(AST_List* node, const UnwindInfo& unw_info) {
         std::vector<CompilerVariable*> elts;
@@ -1766,7 +1754,7 @@ private:
             // I think it's better to just not generate bad speculations:
             if (rtn->canConvertTo(speculated_type)) {
                 auto source = irstate->getSourceInfo();
-                printf("On %s:%d, function %s:\n", source->getFn()->c_str(), source->body[0]->lineno,
+                printf("On %s:%d, function %s:\n", source->getFn()->c_str(), source->getBody()[0]->lineno,
                        source->getName()->c_str());
                 irstate->getSourceInfo()->cfg->print();
             }
@@ -1844,9 +1832,6 @@ private:
             case AST_TYPE::Dict:
                 rtn = evalDict(ast_cast<AST_Dict>(node), unw_info);
                 break;
-            case AST_TYPE::Lambda:
-                rtn = evalLambda(ast_cast<AST_Lambda>(node), unw_info);
-                break;
             case AST_TYPE::List:
                 rtn = evalList(ast_cast<AST_List>(node), unw_info);
                 break;
@@ -3283,7 +3268,7 @@ FunctionMetadata* wrapFunction(AST* node, AST_arguments* args, const std::vector
     FunctionMetadata*& md = made[node];
     if (md == NULL) {
         std::unique_ptr<SourceInfo> si(
-            new SourceInfo(source->parent_module, source->scoping, source->future_flags, node, body, source->getFn()));
+            new SourceInfo(source->parent_module, source->scoping, source->future_flags, node, source->getFn()));
         if (args)
             md = new FunctionMetadata(args->args.size(), args->vararg, args->kwarg, std::move(si));
         else

src/core/ast.cpp

@@ -1405,7 +1405,12 @@ bool PrintVisitor::visit_functiondef(AST_FunctionDef* node) {
         printIndent();
     }
 
-    stream << "def " << node->name.s() << "(";
+    stream << "def ";
+    if (node->name != InternedString())
+        stream << node->name.s();
+    else
+        stream << "<lambda>";
+    stream << "(";
     node->args->accept(this);
     stream << ")";
 

src/core/ast.h

@@ -503,7 +503,8 @@ class AST_Expression : public AST {
 public:
     std::unique_ptr<InternedStringPool> interned_strings;
 
-    AST_expr* body;
+    // this should be an expr but we convert it into a AST_Return(AST_expr) to make the code simpler
+    AST_stmt* body;
 
     virtual void accept(ASTVisitor* v);
 
@@ -542,7 +543,7 @@ class AST_FunctionDef : public AST_stmt {
 public:
     std::vector<AST_stmt*> body;
     std::vector<AST_expr*> decorator_list;
-    InternedString name;
+    InternedString name; // if the name is not set this is a lambda
     AST_arguments* args;
 
     virtual void accept(ASTVisitor* v);

src/core/cfg.cpp

@@ -253,7 +253,7 @@ private:
 
     unsigned int next_var_index = 0;
 
-    friend CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body, const ParamNames& param_names);
+    friend CFG* computeCFG(SourceInfo* source, const ParamNames& param_names);
 
 public:
     CFGVisitor(SourceInfo* source, AST_TYPE::AST_TYPE root_type, FutureFlags future_flags,
@@ -1199,14 +1199,25 @@ private:
     }
 
     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);
-        rtn->lineno = node->lineno;
-        rtn->col_offset = node->col_offset;
-        // lambdas create scope, need to register as replacement
-        scoping_analysis->registerScopeReplacement(node, rtn);
-        return rtn;
+        // we convert lambdas into normal functions (but don't give it a name)
+        auto def = new AST_FunctionDef();
+        def->lineno = node->lineno;
+        def->col_offset = node->col_offset;
+
+        auto stmt = new AST_Return;
+        stmt->lineno = node->lineno;
+        stmt->col_offset = node->col_offset;
+        stmt->value = node->body; // don't remap now; will be CFG'ed later
+
+        def->body = { stmt };
+        def->args = remapArguments(node->args);
+
+        scoping_analysis->registerScopeReplacement(node, def);
+
+        auto tmp = nodeName();
+        pushAssign(tmp, new AST_MakeFunction(def));
+
+        return makeLoad(tmp, def, /* is_kill */ false);
     }
 
     AST_expr* remapLangPrimitive(AST_LangPrimitive* node) {
@@ -2851,9 +2862,11 @@ void VRegInfo::assignVRegs(CFG* cfg, const ParamNames& param_names, ScopeInfo* s
 #endif
 }
 
-CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body, const ParamNames& param_names) {
+CFG* computeCFG(SourceInfo* source, const ParamNames& param_names) {
     STAT_TIMER(t0, "us_timer_computecfg", 0);
 
+    auto body = source->getBody();
+
     CFG* rtn = new CFG();
 
     ScopingAnalysis* scoping_analysis = source->scoping;

src/core/cfg.h

@@ -320,7 +320,7 @@ public:
 };
 
 class SourceInfo;
-CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body, const ParamNames& param_names);
+CFG* computeCFG(SourceInfo* source, const ParamNames& param_names);
 void printCFG(CFG* cfg);
 }
 

src/core/stringpool.h

@@ -75,6 +75,7 @@ public:
         // assert(this->pool == rhs.pool || this->pool == invalidPool() || rhs.pool == invalidPool());
         return this->_str == rhs._str;
     }
+    bool operator!=(InternedString rhs) const { return !(*this == rhs); }
 
     // This function compares the actual string contents
     bool operator<(InternedString rhs) const { return this->s().compare(rhs.s()) == -1; }

src/core/types.h

@@ -421,20 +421,17 @@ public:
     ScopeInfo* getScopeInfo();
     LivenessAnalysis* getLiveness();
 
-    // TODO we're currently copying the body of the AST into here, since lambdas don't really have a statement-based
-    // body and we have to create one.  Ideally, we'd be able to avoid the space duplication for non-lambdas.
-    const std::vector<AST_stmt*> body;
-
     // does not throw CXX or CAPI exceptions:
     BORROWED(BoxedString*) getName() noexcept;
     BORROWED(BoxedString*) getFn();
 
     InternedString mangleName(InternedString id);
 
+    llvm::ArrayRef<AST_stmt*> getBody() const;
+
     Box* getDocString();
 
-    SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast,
-               std::vector<AST_stmt*> body, BoxedString* fn);
+    SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast, BoxedString* fn);
     ~SourceInfo();
 
 private:

src/runtime/builtin_modules/thread.cpp

@@ -161,7 +161,7 @@ public:
 
     static Box* exit(Box* _self, Box* arg1, Box* arg2, Box** args) { return release(_self); }
 
-    static void threadLockDestructor(Box* _self) {
+    static void dealloc(Box* _self) {
         RELEASE_ASSERT(_self->cls == thread_lock_cls, "");
         BoxedThreadLock* self = static_cast<BoxedThreadLock*>(_self);
 
@@ -173,6 +173,8 @@ public:
             PyThread_free_lock(self->lock_lock);
             self->lock_lock = NULL;
         }
+
+        _self->cls->tp_free(_self);
     }
 
     static Box* locked(Box* _self) {
@@ -185,8 +187,6 @@ public:
         }
         Py_RETURN_TRUE;
     }
-
-    static void dealloc(Box* b) noexcept { Py_FatalError("unimplemented"); }
 };
 
 
@@ -232,7 +232,6 @@ void setupThread() {
 
     thread_lock_cls = BoxedClass::create(type_cls, object_cls, 0, 0, sizeof(BoxedThreadLock), false, "lock", true,
                                          BoxedThreadLock::dealloc, NULL, false);
-    thread_lock_cls->tp_dealloc = BoxedThreadLock::threadLockDestructor;
     thread_lock_cls->instances_are_nonzero = true;
 
     thread_lock_cls->giveAttr("__module__", boxString("thread"));

src/runtime/long.cpp

@@ -65,6 +65,7 @@ int _PyLong_DigitValue[256] = {
 
 void BoxedLong::tp_dealloc(Box* b) noexcept {
     mpz_clear(static_cast<BoxedLong*>(b)->n);
+    b->cls->tp_free(b);
 }
 
 extern "C" int _PyLong_Sign(PyObject* l) noexcept {

src/runtime/util.cpp

@@ -251,7 +251,7 @@ extern "C" void dumpEx(void* p, int levels) {
             FunctionMetadata* md = f->md;
             if (md->source) {
                 printf("User-defined function '%s'\n", md->source->getName()->c_str());
-                printf("Defined at %s:%d\n", md->source->getFn()->c_str(), md->source->body[0]->lineno);
+                printf("Defined at %s:%d\n", md->source->getFn()->c_str(), md->source->getBody()[0]->lineno);
 
                 if (md->source->cfg && levels > 0) {
                     md->source->cfg->print();

test/unittests/analysis.cpp

@@ -40,11 +40,10 @@ TEST_F(AnalysisTest, augassign) {
 
     FutureFlags future_flags = getFutureFlags(module->body, fn.c_str());
 
-    SourceInfo* si = new SourceInfo(createModule(boxString("augassign"), fn.c_str()), scoping, future_flags, func,
-            func->body, boxString(fn));
+    SourceInfo* si = new SourceInfo(createModule(boxString("augassign"), fn.c_str()), scoping, future_flags, func, boxString(fn));
 
     ParamNames param_names(si->ast, si->getInternedStrings());
-    CFG* cfg = computeCFG(si, func->body, param_names);
+    CFG* cfg = computeCFG(si, param_names);
     std::unique_ptr<LivenessAnalysis> liveness = computeLivenessInfo(cfg);
     auto&& vregs = cfg->getVRegInfo();
 
@@ -73,10 +72,10 @@ void doOsrTest(bool is_osr, bool i_maybe_undefined) {
 
     ScopeInfo* scope_info = scoping->getScopeInfoForNode(func);
     std::unique_ptr<SourceInfo> si(new SourceInfo(createModule(boxString("osr" + std::to_string((is_osr << 1) + i_maybe_undefined)),
-                    fn.c_str()), scoping, future_flags, func, func->body, boxString(fn)));
+                    fn.c_str()), scoping, future_flags, func, boxString(fn)));
     FunctionMetadata* clfunc = new FunctionMetadata(0, false, false, std::move(si));
 
-    CFG* cfg = computeCFG(clfunc->source.get(), func->body, clfunc->param_names);
+    CFG* cfg = computeCFG(clfunc->source.get(), clfunc->param_names);
     clfunc->source->cfg = cfg;
     std::unique_ptr<LivenessAnalysis> liveness = computeLivenessInfo(cfg);