Allow the re-usage of AST nodes for now, though it's muddies some things

src/analysis/scoping_analysis.cpp

@@ -335,16 +335,16 @@ void ScopingAnalysis::processNameUsages(ScopingAnalysis::NameUsageMap* usages) {
 
 ScopeInfo* ScopingAnalysis::analyzeSubtree(AST* node) {
 #ifndef NDEBUG
-    std::vector<AST*>* flattened = flatten(parent_module->body, false);
+    std::vector<AST*> flattened;
+    flatten(parent_module->body, flattened, false);
     bool found = 0;
-    for (int i = 0; i < flattened->size(); i++) {
-        if ((*flattened)[i] == node) {
+    for (AST* n : flattened) {
+        if (n == node) {
             found = true;
             break;
         }
     }
     assert(found);
-    delete flattened;
 #endif
 
     NameUsageMap usages;

src/codegen/irgen/irgenerator.cpp

@@ -188,17 +188,9 @@ class IRGeneratorImpl : public IRGenerator {
         }
 
     private:
-#ifndef NDEBUG
-        std::unordered_set<AST*> gotten_op_infos;
-#endif
         OpInfo getOpInfoForNode(AST* ast) {
             assert(ast);
 
-#ifndef NDEBUG
-            assert(gotten_op_infos.count(ast) == 0);
-            gotten_op_infos.insert(ast);
-#endif
-
             EffortLevel::EffortLevel effort = irstate->getEffortLevel();
             bool record_types = (effort != EffortLevel::INTERPRETED && effort != EffortLevel::MAXIMAL);
 

src/core/ast.cpp

@@ -1214,6 +1214,7 @@ class FlattenVisitor : public ASTVisitor {
         virtual bool visit_expr(AST_Expr *node) { output->push_back(node); return false; }
         virtual bool visit_for(AST_For *node) { output->push_back(node); return !expand_scopes; }
         virtual bool visit_functiondef(AST_FunctionDef *node) { output->push_back(node); return !expand_scopes; }
+        virtual bool visit_global(AST_Global *node) { output->push_back(node); return false; }
         virtual bool visit_if(AST_If *node) { output->push_back(node); return false; }
         virtual bool visit_import(AST_Import *node) { output->push_back(node); return false; }
         virtual bool visit_index(AST_Index *node) { output->push_back(node); return false; }
@@ -1239,22 +1240,18 @@ class FlattenVisitor : public ASTVisitor {
         virtual bool visit_clsattribute(AST_ClsAttribute *node) { output->push_back(node); return false; }
 };
 
-std::vector<AST*>* flatten(std::vector<AST_stmt*> &roots, bool expand_scopes) {
-    std::vector<AST*> *rtn = new std::vector<AST*>();
-    FlattenVisitor visitor(rtn, expand_scopes);
+void flatten(const std::vector<AST_stmt*> &roots, std::vector<AST*> &output, bool expand_scopes) {
+    FlattenVisitor visitor(&output, expand_scopes);
 
     for (int i = 0; i < roots.size(); i++) {
         roots[i]->accept(&visitor);
     }
-    return rtn;
 }
 
-std::vector<AST*>* flatten(AST_expr* root, bool expand_scopes) {
-    std::vector<AST*> *rtn = new std::vector<AST*>();
-    FlattenVisitor visitor(rtn, expand_scopes);
+void flatten(AST_expr* root, std::vector<AST*> &output, bool expand_scopes) {
+    FlattenVisitor visitor(&output, expand_scopes);
 
     root->accept(&visitor);
-    return rtn;
 }
 
 }

src/core/ast.h

@@ -842,20 +842,17 @@ class PrintVisitor : public ASTVisitor {
 // Given an AST node, return a vector of the node plus all its descendents.
 // This is useful for analyses that care more about the constituent nodes than the
 // exact tree structure; ex, finding all "global" directives.
-std::vector<AST*>* flatten(std::vector<AST_stmt*> &roots, bool expand_scopes);
-std::vector<AST*>* flatten(AST_expr *root, bool expand_scopes);
+void flatten(const std::vector<AST_stmt*> &roots, std::vector<AST*> &output, bool expand_scopes);
+void flatten(AST_expr *root, std::vector<AST*> &output, bool expand_scopes);
 // Similar to the flatten() function, but filters for a specific type of ast nodes:
 template <class T, class R>
-std::vector<T*>* findNodes(const R &roots, bool expand_scopes) {
-    std::vector<T*> *rtn = new std::vector<T*>();
-    std::vector<AST*> *flattened = flatten(roots, expand_scopes);
-    for (int i = 0; i < flattened->size(); i++) {
-        AST* n = (*flattened)[i];
+void findNodes(const R &roots, std::vector<T*> &output, bool expand_scopes) {
+    std::vector<AST*> flattened;
+    flatten(roots, flattened, expand_scopes);
+    for (AST* n : flattened) {
         if (n->type == T::TYPE)
-            rtn->push_back(reinterpret_cast<T*>(n));
+            output.push_back(reinterpret_cast<T*>(n));
     }
-    delete flattened;
-    return rtn;
 }
 
 };

src/core/cfg.cpp

@@ -977,16 +977,6 @@ class CFGVisitor : public ASTVisitor {
 
             AST_expr *hasnext_attr = makeLoadAttribute(makeName(itername_buf, AST_TYPE::Load), "__hasnext__", true);
             AST_expr *next_attr = makeLoadAttribute(makeName(itername_buf, AST_TYPE::Load), "next", true);
-//#define SAVE_ATTRS
-#ifdef SAVE_ATTRS
-            char hasnextname_buf[80];
-            snprintf(hasnextname_buf, 80, "#hasnext_%p", node);
-            AST_stmt *hasnext_assign = makeAssign(hasnextname_buf, hasnext_attr);
-            push_back(hasnext_assign);
-            char nextname_buf[80];
-            snprintf(nextname_buf, 80, "#next_%p", node);
-            push_back(makeAssign(nextname_buf, next_attr));
-#endif
 
             CFGBlock *test_block = cfg->addBlock();
             AST_Jump* jump_to_test = makeJump();
@@ -995,11 +985,7 @@ class CFGVisitor : public ASTVisitor {
             curblock->connectTo(test_block);
             curblock = test_block;
 
-#ifdef SAVE_ATTRS
-            AST_expr *test_call = makeCall(makeName(hasnextname_buf, AST_TYPE::Load));
-#else
             AST_expr *test_call = makeCall(hasnext_attr);
-#endif
             AST_Branch *test_br = makeBranch(test_call);
             push_back(test_br);
 
@@ -1031,11 +1017,7 @@ class CFGVisitor : public ASTVisitor {
             pushLoop(test_block, end_block);
 
             curblock = loop_block;
-#ifdef SAVE_ATTRS
-            push_back(makeAssign(node->target, makeCall(makeName(nextname_buf, AST_TYPE::Load))));
-#else
             push_back(makeAssign(node->target, makeCall(next_attr)));
-#endif
 
             for (int i = 0; i < node->body.size(); i++) {
                 node->body[i]->accept(this);
@@ -1043,11 +1025,7 @@ class CFGVisitor : public ASTVisitor {
             popLoop();
 
             if (curblock) {
-#ifdef SAVE_ATTRS
-                AST_expr *end_call = makeCall(makeName(hasnextname_buf, AST_TYPE::Load));
-#else
                 AST_expr *end_call = makeCall(hasnext_attr);
-#endif
                 AST_Branch *end_br = makeBranch(end_call);
                 push_back(end_br);
 
@@ -1246,6 +1224,7 @@ CFG* computeCFG(AST_TYPE::AST_TYPE root_type, std::vector<AST_stmt*> body) {
 
     //rtn->print();
 
+#ifndef NDEBUG
     ////
     // Check some properties expected by later stages:
 
@@ -1289,6 +1268,32 @@ CFG* computeCFG(AST_TYPE::AST_TYPE root_type, std::vector<AST_stmt*> body) {
         assert(rtn->blocks[i]->predecessors[0]->idx < i);
     }
 
+    /*
+    // I keep on going back and forth about whether or not it's ok to reuse AST nodes.
+    // On the one hand, it's nice to say that an AST* pointer uniquely identifies a spot
+    // in the computation, but then again the sharing can actually be nice because
+    // it indicates that there are certain similarities between the points, and things such
+    // as type recorders will end up being shared.
+    std::vector<AST*> all_ast_nodes;
+    for (CFGBlock* block : rtn->blocks) {
+        flatten(block->body, all_ast_nodes, false);
+    }
+
+    std::unordered_set<AST*> seen_ast_nodes;
+    for (AST* n : all_ast_nodes) {
+        if (seen_ast_nodes.count(n)) {
+            rtn->print();
+
+            printf("This node appears multiple times in the tree:\n");
+            print_ast(n);
+            printf("\n");
+            assert(0);
+        }
+        seen_ast_nodes.insert(n);
+    }
+    */
+#endif
+
     return rtn;
 }