Merge pull request #407 from tjhance/locals-globals

src/analysis/scoping_analysis.cpp

@@ -105,6 +105,70 @@ public:
 
     bool isPassedToViaClosure(InternedString name) override { return false; }
 
+    bool areLocalsFromModule() override { return true; }
+
+    InternedString mangleName(InternedString id) override { return id; }
+    InternedString internString(llvm::StringRef s) override { abort(); }
+};
+
+typedef llvm::DenseSet<InternedString> StrSet;
+
+// Handles the scope in eval or exec
+// For example for exec, if you write
+// exec "global a ; print a ; print b"
+// It will give `a` the GLOBAL scope type and `b` the NAME type.
+// (For eval, you can't have global statements, so it will just
+// mark everything NAME.)
+class EvalExprScopeInfo : public ScopeInfo {
+private:
+    StrSet forced_globals;
+
+    struct GlobalStmtVisitor : NoopASTVisitor {
+        StrSet& result;
+        GlobalStmtVisitor(StrSet& result) : result(result) {}
+
+        bool visit_functiondef(AST_FunctionDef*) override { return true; }
+        bool visit_classdef(AST_ClassDef*) override { return true; }
+
+        bool visit_global(AST_Global* global_stmt) override {
+            for (InternedString name : global_stmt->names) {
+                result.insert(name);
+            }
+            return true;
+        }
+    };
+
+public:
+    EvalExprScopeInfo() {}
+
+    EvalExprScopeInfo(AST* node) {
+        // Find all the global statements in the node's scope (not delving into FuncitonDefs
+        // or ClassDefs) and put the names in `forced_globals`.
+        GlobalStmtVisitor visitor(forced_globals);
+        node->accept(&visitor);
+    }
+
+    ScopeInfo* getParent() override { return NULL; }
+
+    bool createsClosure() override { return false; }
+    bool takesClosure() override { return false; }
+    bool passesThroughClosure() override { return false; }
+
+    VarScopeType getScopeTypeOfName(InternedString name) override {
+        if (isCompilerCreatedName(name))
+            return VarScopeType::FAST;
+        else if (forced_globals.find(name) != forced_globals.end())
+            return VarScopeType::GLOBAL;
+        else
+            return VarScopeType::NAME;
+    }
+
+    bool usesNameLookup() override { return true; }
+
+    bool isPassedToViaClosure(InternedString name) override { return false; }
+
+    bool areLocalsFromModule() override { return false; }
+
     InternedString mangleName(InternedString id) override { return id; }
     InternedString internString(llvm::StringRef s) override { abort(); }
 };
@@ -115,8 +179,6 @@ struct ScopingAnalysis::ScopeNameUsage {
     const std::string* private_name;
     ScopingAnalysis* scoping;
 
-    typedef llvm::DenseSet<InternedString> StrSet;
-
     // Properties determined from crawling the scope:
     StrSet read;
     StrSet written;
@@ -245,6 +307,8 @@ public:
         return usage->got_from_closure.count(name) > 0 || usage->passthrough_accesses.count(name) > 0;
     }
 
+    bool areLocalsFromModule() override { return false; }
+
     InternedString mangleName(const InternedString id) override {
         return pyston::mangleName(id, usage->private_name, usage->scoping->getInternedStrings());
     }
@@ -644,8 +708,6 @@ void ScopingAnalysis::processNameUsages(ScopingAnalysis::NameUsageMap* usages) {
         ScopeInfo* parent_info = this->scopes[(usage->parent == NULL) ? this->parent_module : usage->parent->node];
 
         switch (node->type) {
-            case AST_TYPE::Expression:
-            case AST_TYPE::Suite:
             case AST_TYPE::ClassDef: {
                 ScopeInfoBase* scopeInfo
                     = new ScopeInfoBase(parent_info, usage, usage->node, true /* usesNameLookup */);
@@ -719,12 +781,11 @@ ScopingAnalysis::ScopingAnalysis(AST_Module* m) : parent_module(m), interned_str
 }
 
 ScopingAnalysis::ScopingAnalysis(AST_Expression* e) : interned_strings(*e->interned_strings.get()) {
-    auto scope_info = getScopeInfoForNode(e);
-    scopes[e] = scope_info;
+    // It's an expression, so it can't have a `global` statement
+    scopes[e] = new EvalExprScopeInfo();
 }
 
 ScopingAnalysis::ScopingAnalysis(AST_Suite* s) : interned_strings(*s->interned_strings.get()) {
-    auto scope_info = getScopeInfoForNode(s);
-    scopes[s] = scope_info;
+    scopes[s] = new EvalExprScopeInfo(s);
 }
 }

src/analysis/scoping_analysis.h

@@ -114,6 +114,8 @@ public:
     // `exec` or `eval` scope.
     virtual bool usesNameLookup() = 0;
 
+    virtual bool areLocalsFromModule() = 0;
+
     virtual InternedString mangleName(InternedString id) = 0;
     virtual InternedString internString(llvm::StringRef) = 0;
 };

src/codegen/ast_interpreter.cpp

@@ -1094,15 +1094,7 @@ Value ASTInterpreter::visit_name(AST_Name* node) {
             return Value();
         }
         case ScopeInfo::VarScopeType::NAME: {
-            assert(frame_info.boxedLocals->cls == dict_cls);
-            auto& d = static_cast<BoxedDict*>(frame_info.boxedLocals)->d;
-            auto it = d.find(boxString(node->id.str()));
-            if (it != d.end()) {
-                Box* value = it->second;
-                return value;
-            }
-
-            return getGlobal(source_info->parent_module, &node->id.str());
+            return boxedLocalsGet(frame_info.boxedLocals, node->id.c_str(), source_info->parent_module);
         }
         default:
             abort();
@@ -1169,7 +1161,6 @@ Box* astInterpretFunctionEval(CompiledFunction* cf, Box* boxedLocals) {
 
     ASTInterpreter interpreter(cf);
     interpreter.initArguments(0, NULL, NULL, NULL, NULL, NULL, NULL);
-    RELEASE_ASSERT(boxedLocals->cls == dict_cls, "we don't support non-dicts here yet");
     interpreter.setBoxedLocals(boxedLocals);
     Value v = ASTInterpreter::execute(interpreter);
 

src/codegen/unwinding.cpp

@@ -638,12 +638,20 @@ Box* fastLocalsToBoxedLocals() {
     for (PythonFrameIterator& frame_iter : unwindPythonFrames()) {
         BoxedDict* d;
         BoxedClosure* closure;
-        CompiledFunction* cf;
         FrameInfo* frame_info;
+
+        CompiledFunction* cf = frame_iter.getCF();
+        ScopeInfo* scope_info = cf->clfunc->source->getScopeInfo();
+
+        if (scope_info->areLocalsFromModule()) {
+            // TODO we should cache this in frame_info->locals or something so that locals()
+            // (and globals() too) will always return the same dict
+            return makeAttrWrapper(getCurrentModule());
+        }
+
         if (frame_iter.getId().type == PythonFrameId::COMPILED) {
             d = new BoxedDict();
 
-            cf = frame_iter.getCF();
             uint64_t ip = frame_iter.getId().ip;
 
             assert(ip > cf->code_start);
@@ -727,7 +735,6 @@ Box* fastLocalsToBoxedLocals() {
         } else if (frame_iter.getId().type == PythonFrameId::INTERPRETED) {
             d = localsForInterpretedFrame((void*)frame_iter.getId().bp, true);
             closure = passedClosureForInterpretedFrame((void*)frame_iter.getId().bp);
-            cf = getCFForInterpretedFrame((void*)frame_iter.getId().bp);
             frame_info = getFrameInfoForInterpretedFrame((void*)frame_iter.getId().bp);
         } else {
             abort();
@@ -747,7 +754,6 @@ Box* fastLocalsToBoxedLocals() {
                 const std::string& name = attr_offset.first();
                 int offset = attr_offset.second;
                 Box* val = closure->attrs.attr_list->attrs[offset];
-                ScopeInfo* scope_info = cf->clfunc->source->getScopeInfo();
                 if (val != NULL && scope_info->isPassedToViaClosure(scope_info->internString(name))) {
                     Box* boxedName = boxString(name);
                     if (d->d.count(boxedName) == 0) {

src/runtime/objmodel.cpp

@@ -4441,21 +4441,35 @@ Box* coerceUnicodeToStr(Box* unicode) {
     return r;
 }
 
+// TODO Make these fast, do inline caches and stuff
+
 extern "C" void boxedLocalsSet(Box* boxedLocals, const char* attr, Box* val) {
     setitem(boxedLocals, boxString(attr), val);
 }
 
 extern "C" Box* boxedLocalsGet(Box* boxedLocals, const char* attr, BoxedModule* parent_module) {
     assert(parent_module->cls == module_cls);
-
     assert(boxedLocals != NULL);
-    RELEASE_ASSERT(boxedLocals->cls == dict_cls, "we don't support non-dict here yet");
+
+    if (boxedLocals->cls == dict_cls) {
         auto& d = static_cast<BoxedDict*>(boxedLocals)->d;
         auto it = d.find(boxString(attr));
         if (it != d.end()) {
             Box* value = it->second;
             return value;
         }
+    } else {
+        try {
+            return getitem(boxedLocals, boxString(attr));
+        } catch (ExcInfo e) {
+            // TODO should check the exact semantic here but it's something like:
+            // If it throws a KeyError, then the variable doesn't exist so move on
+            // and check the globals (below); otherwise, just propogate the exception.
+            if (!isInstance(e.value, KeyError)) {
+                throw;
+            }
+        }
+    }
 
     // TODO exception name?
     std::string attr_string(attr);

test/tests/eval_test.py

-# TODO lots of eval functionality not implemented
-
 print eval("3 + 4")
 
 a = 5
 print eval("a")
 
-#print eval("[b for b in range(5)]")
-#print b
+print eval("[b for b in range(5)]")
+print b
 
-#c = 2
-#print eval("[c for c in range(5)]")
-#print c
+c = 2
+print eval("[c for c in range(5)]")
+print c
 
-#try:
-#    print eval("int('abc')")
-#except ValueError:
-#    print 'got ValueError'
+try:
+    print eval("int('abc')")
+except ValueError:
+    print 'got ValueError'
 
 d = 19
 e = 20
@@ -85,10 +83,10 @@ o = 300
 print 'eval eval o', eval("eval('o')")
 
 # This works in the global scope but not in the local scope, because o1 is a global:
-#print eval("[(lambda p1 : p1 + o1)(5) for o1 in range(5)]")
+print eval("[(lambda p1 : p1 + o1)(5) for o1 in range(5)]")
 def lambda_func():
     try:
-        pass #print eval("[(lambda p2 : p2 + o2)(5) for o2 in range(5)]")
+        print eval("[(lambda p2 : p2 + o2)(5) for o2 in range(5)]")
     except NameError as e:
         print e.message
 lambda_func()
@@ -106,9 +104,9 @@ def func2():
 
     print 'shadow3', eval("shadow3 + sum([2 for shadow3 in range(5)]) + shadow3")
 func2()
-#print 'shadow1', shadow2
-#print 'shadow2', shadow2
-#print 'shadow3', shadow3
+print 'shadow1', shadow2
+print 'shadow2', shadow2
+print 'shadow3', shadow3
 
 
 def func3():
@@ -123,16 +121,12 @@ def func3():
         print 'NameError', e.message
 func3()
 
-"""
-
 changing_global = -1
 def print_changing_global():
     print 'changing_global is', changing_global
     return 0
 eval("[print_changing_global() for changing_global in range(5)]")
 
-"""
-
 def do_changing_local():
     # this won't get modified:
     changing_local = -1

test/tests/globals_func.py

@@ -18,3 +18,8 @@ except NameError:
 # You're allowed to assign through globals and have it affect the module:
 globals()['x'] = 1
 print x
+
+# locals should do the same as globals
+print locals()['x']
+locals()['x'] = 2
+print x