move ScopeInfo ownership to SourceInfo

src/analysis/scoping_analysis.cpp

@@ -903,13 +903,13 @@ void ScopingAnalysis::processNameUsages(ScopingAnalysis::NameUsageMap* usages) {
         ScopeNameUsage* usage = sorted_usages[i];
         AST* node = usage->node;
 
-        ScopeInfo* parent_info = this->scopes[(usage->parent == NULL) ? this->parent_module : usage->parent->node];
+        ScopeInfo* parent_info
+            = this->scopes[(usage->parent == NULL) ? this->parent_module : usage->parent->node].get();
 
         switch (node->type) {
             case AST_TYPE::ClassDef: {
-                ScopeInfoBase* scopeInfo = new ScopeInfoBase(parent_info, usage, usage->node, true /* usesNameLookup */,
-                                                             globals_from_module);
-                this->scopes[node] = scopeInfo;
+                this->scopes[node] = llvm::make_unique<ScopeInfoBase>(parent_info, usage, usage->node,
+                                                                      true /* usesNameLookup */, globals_from_module);
                 break;
             }
             case AST_TYPE::FunctionDef:
@@ -917,10 +917,9 @@ void ScopingAnalysis::processNameUsages(ScopingAnalysis::NameUsageMap* usages) {
             case AST_TYPE::GeneratorExp:
             case AST_TYPE::DictComp:
             case AST_TYPE::SetComp: {
-                ScopeInfoBase* scopeInfo
-                    = new ScopeInfoBase(parent_info, usage, usage->node,
-                                        usage->hasNameForcingSyntax() /* usesNameLookup */, globals_from_module);
-                this->scopes[node] = scopeInfo;
+                this->scopes[node] = llvm::make_unique<ScopeInfoBase>(
+                    parent_info, usage, usage->node, usage->hasNameForcingSyntax() /* usesNameLookup */,
+                    globals_from_module);
                 break;
             }
             default:
@@ -934,14 +933,15 @@ InternedStringPool& ScopingAnalysis::getInternedStrings() {
     return *interned_strings;
 }
 
-ScopeInfo* ScopingAnalysis::analyzeSubtree(AST* node) {
+std::unique_ptr<ScopeInfo> ScopingAnalysis::analyzeSubtree(AST* node) {
     NameUsageMap usages;
     usages[node] = new ScopeNameUsage(node, NULL, this);
     NameCollectorVisitor::collect(node, &usages, this);
 
     processNameUsages(&usages);
 
-    ScopeInfo* rtn = scopes[node];
+    std::unique_ptr<ScopeInfo> rtn = std::move(scopes[node]);
+    // scopes.erase(node);
     assert(rtn);
     return rtn;
 }
@@ -959,17 +959,17 @@ void ScopingAnalysis::registerScopeReplacement(AST* original_node, AST* new_node
     scope_replacements[new_node] = original_node;
 }
 
-ScopeInfo* ScopingAnalysis::getScopeInfoForNode(AST* node) {
+std::unique_ptr<ScopeInfo> ScopingAnalysis::getScopeInfoForNode(AST* node) {
     assert(node);
 
     auto it = scope_replacements.find(node);
     if (it != scope_replacements.end())
         node = it->second;
 
-    auto rtn = scopes.find(node);
-    if (rtn != scopes.end()) {
-        assert(rtn->second);
-        return rtn->second;
+    if (scopes.count(node)) {
+        std::unique_ptr<ScopeInfo> rtn = std::move(scopes[node]);
+        assert(rtn);
+        return rtn;
     }
 
     return analyzeSubtree(node);
@@ -993,10 +993,10 @@ ScopingAnalysis::ScopingAnalysis(AST* ast, bool globals_from_module)
 
     if (globals_from_module) {
         assert(ast->type == AST_TYPE::Module);
-        scopes[ast] = new ModuleScopeInfo();
+        scopes[ast] = llvm::make_unique<ModuleScopeInfo>();
         parent_module = static_cast<AST_Module*>(ast);
     } else {
-        scopes[ast] = new EvalExprScopeInfo(ast, globals_from_module);
+        scopes[ast] = llvm::make_unique<EvalExprScopeInfo>(ast, globals_from_module);
     }
 }
 }

src/analysis/scoping_analysis.h

@@ -151,13 +151,13 @@ public:
     typedef llvm::DenseMap<AST*, ScopeNameUsage*> NameUsageMap;
 
 private:
-    llvm::DenseMap<AST*, ScopeInfo*> scopes;
+    llvm::DenseMap<AST*, std::unique_ptr<ScopeInfo>> scopes;
     AST_Module* parent_module;
     InternedStringPool* interned_strings;
 
     llvm::DenseMap<AST*, AST*> scope_replacements;
 
-    ScopeInfo* analyzeSubtree(AST* node);
+    std::unique_ptr<ScopeInfo> analyzeSubtree(AST* node);
     void processNameUsages(NameUsageMap* usages);
 
     bool globals_from_module;
@@ -173,7 +173,7 @@ public:
     void registerScopeReplacement(AST* original_node, AST* new_node);
 
     ScopingAnalysis(AST* ast, bool globals_from_module);
-    ScopeInfo* getScopeInfoForNode(AST* node);
+    std::unique_ptr<ScopeInfo> getScopeInfoForNode(AST* node);
 
     InternedStringPool& getInternedStrings();
     bool areGlobalsFromModule() { return globals_from_module; }

src/codegen/ast_interpreter.cpp

@@ -1159,19 +1159,7 @@ Value ASTInterpreter::createFunction(AST* node, AST_arguments* args) {
     u.d.ptr = &defaults[0];
     u.d.s = defaults.size() - 1;
 
-    bool takes_closure;
-    source_info->scoping->getScopeInfoForNode(node);
-
-    // Optimization: when compiling a module, it's nice to not have to run analyses into the
-    // entire module's source code.
-    // If we call getScopeInfoForNode, that will trigger an analysis of that function tree,
-    // but we're only using it here to figure out if that function takes a closure.
-    // Top level functions never take a closure, so we can skip the analysis.
-    if (source_info->ast->type == AST_TYPE::Module)
-        takes_closure = false;
-    else {
-        takes_closure = source_info->scoping->getScopeInfoForNode(node)->takesClosure();
-    }
+    bool takes_closure = md->source->getScopeInfo()->takesClosure();
 
     BoxedClosure* closure = 0;
     RewriterVar* closure_var = NULL;
@@ -1238,8 +1226,7 @@ Value ASTInterpreter::visit_makeFunction(AST_MakeFunction* mkfn) {
 Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) {
     abortJITing();
     AST_ClassDef* node = mkclass->class_def;
-    ScopeInfo* scope_info = source_info->scoping->getScopeInfoForNode(node);
-    assert(scope_info);
+
 
     BoxedTuple* basesTuple = BoxedTuple::create(node->bases.size());
     AUTO_DECREF(basesTuple);
@@ -1253,6 +1240,11 @@ Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) {
     for (AST_expr* d : node->decorator_list)
         decorators.push_back(visit_expr(d).o);
 
+    FunctionMetadata* md = wrapFunction(node, nullptr, source_info);
+
+    ScopeInfo* scope_info = md->source->getScopeInfo();
+    assert(scope_info);
+
     BoxedClosure* closure = NULL;
     if (scope_info->takesClosure()) {
         if (this->scope_info->passesThroughClosure())
@@ -1261,7 +1253,6 @@ Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) {
             closure = created_closure;
         assert(closure);
     }
-    FunctionMetadata* md = wrapFunction(node, nullptr, source_info);
 
     Box* passed_globals = NULL;
     if (!getMD()->source->scoping->areGlobalsFromModule())

src/codegen/codegen.cpp

@@ -92,7 +92,12 @@ void FunctionMetadata::addVersion(CompiledFunction* compiled) {
 }
 
 SourceInfo::SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast, BoxedString* fn)
-    : parent_module(m), scoping(scoping), scope_info(NULL), ast(ast), cfg(NULL), future_flags(future_flags) {
+    : parent_module(m),
+      scoping(scoping),
+      scope_info(scoping->getScopeInfoForNode(ast)),
+      ast(ast),
+      cfg(NULL),
+      future_flags(future_flags) {
     assert(fn);
 
     // TODO: this is a very bad way of handling this:

src/codegen/irgen/hooks.cpp

@@ -183,9 +183,8 @@ Box* SourceInfo::getDocString() {
 }
 
 ScopeInfo* SourceInfo::getScopeInfo() {
-    if (!scope_info)
-        scope_info = scoping->getScopeInfoForNode(ast);
-    return scope_info;
+    assert(scope_info);
+    return scope_info.get();
 }
 
 LivenessAnalysis* SourceInfo::getLiveness() {

src/codegen/irgen/irgenerator.cpp

@@ -375,11 +375,6 @@ ScopeInfo* IRGenState::getScopeInfo() {
     return getSourceInfo()->getScopeInfo();
 }
 
-ScopeInfo* IRGenState::getScopeInfoForNode(AST* node) {
-    auto source = getSourceInfo();
-    return source->scoping->getScopeInfoForNode(node);
-}
-
 llvm::Value* IRGenState::getGlobals() {
     assert(globals);
     return globals;
@@ -1576,8 +1571,7 @@ private:
     CompilerVariable* evalMakeClass(AST_MakeClass* mkclass, const UnwindInfo& unw_info) {
         assert(mkclass->type == AST_TYPE::MakeClass && mkclass->class_def->type == AST_TYPE::ClassDef);
         AST_ClassDef* node = mkclass->class_def;
-        ScopeInfo* scope_info = irstate->getScopeInfoForNode(node);
-        assert(scope_info);
+
 
         std::vector<CompilerVariable*> bases;
         for (auto b : node->bases) {
@@ -1594,6 +1588,8 @@ private:
         }
 
         FunctionMetadata* md = wrapFunction(node, nullptr, irstate->getSourceInfo());
+        ScopeInfo* scope_info = md->source->getScopeInfo();
+        assert(scope_info);
 
         // TODO duplication with _createFunction:
         llvm::Value* this_closure = NULL;
@@ -1644,19 +1640,7 @@ private:
             defaults.push_back(converted);
         }
 
-        bool takes_closure;
-        // Optimization: when compiling a module, it's nice to not have to run analyses into the
-        // entire module's source code.
-        // If we call getScopeInfoForNode, that will trigger an analysis of that function tree,
-        // but we're only using it here to figure out if that function takes a closure.
-        // Top level functions never take a closure, so we can skip the analysis.
-        if (irstate->getSourceInfo()->ast->type == AST_TYPE::Module)
-            takes_closure = false;
-        else {
-            takes_closure = irstate->getScopeInfoForNode(node)->takesClosure();
-        }
-
-        bool is_generator = md->source->is_generator;
+        bool takes_closure = md->source->scope_info->takesClosure();
 
         llvm::Value* this_closure = NULL;
         if (takes_closure) {

src/codegen/irgen/irgenerator.h

@@ -119,7 +119,6 @@ public:
     PhiAnalysis* getPhis() { return phis.get(); }
 
     ScopeInfo* getScopeInfo();
-    ScopeInfo* getScopeInfoForNode(AST* node);
 
     llvm::MDNode* getFuncDbgInfo() { return func_dbg_info; }
 

src/core/types.h

@@ -443,7 +443,7 @@ private:
 public:
     BoxedModule* parent_module;
     ScopingAnalysis* scoping;
-    ScopeInfo* scope_info;
+    std::unique_ptr<ScopeInfo> scope_info;
     AST* ast;
     CFG* cfg;
     FutureFlags future_flags;

test/unittests/analysis.cpp

@@ -36,14 +36,14 @@ TEST_F(AnalysisTest, augassign) {
     assert(module->body[0]->type == AST_TYPE::FunctionDef);
     AST_FunctionDef* func = static_cast<AST_FunctionDef*>(module->body[0]);
 
-    ScopeInfo* scope_info = scoping->getScopeInfoForNode(func);
-    ASSERT_FALSE(scope_info->getScopeTypeOfName(module->interned_strings->get("a")) == ScopeInfo::VarScopeType::GLOBAL);
-    ASSERT_FALSE(scope_info->getScopeTypeOfName(module->interned_strings->get("b")) == ScopeInfo::VarScopeType::GLOBAL);
-
     FutureFlags future_flags = getFutureFlags(module->body, fn.c_str());
 
     SourceInfo* si = new SourceInfo(createModule(boxString("augassign"), fn.c_str()), scoping, future_flags, func, boxString(fn));
 
+    ScopeInfo* scope_info = si->getScopeInfo();
+    ASSERT_FALSE(scope_info->getScopeTypeOfName(module->interned_strings->get("a")) == ScopeInfo::VarScopeType::GLOBAL);
+    ASSERT_FALSE(scope_info->getScopeTypeOfName(module->interned_strings->get("b")) == ScopeInfo::VarScopeType::GLOBAL);
+
     ParamNames param_names(si->ast, si->getInternedStrings());
     CFG* cfg = computeCFG(si, param_names);
     std::unique_ptr<LivenessAnalysis> liveness = computeLivenessInfo(cfg);
@@ -72,9 +72,9 @@ void doOsrTest(bool is_osr, bool i_maybe_undefined) {
 
     FutureFlags future_flags = getFutureFlags(module->body, fn.c_str());
 
-    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, boxString(fn)));
+    ScopeInfo* scope_info = si->getScopeInfo();
     FunctionMetadata* clfunc = new FunctionMetadata(0, false, false, std::move(si));
 
     CFG* cfg = computeCFG(clfunc->source.get(), clfunc->param_names);