exec and eval compilations are always areGlobalsFromModule() == false

Before, if we saw that they were going to immediately be executed in
the current globals scope (ie no "in" clause), we would execute them
without custom globals.

In order to split the compilation from the execution, just always
compile them with custom globals support.  This involved some refactoring
of the assumptions about when the globals object gets passed or not.

src/codegen/ast_interpreter.cpp

@@ -767,8 +767,11 @@ Box* ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::v
         }
         assert(closure);
     }
-    return boxCLFunction(cl, closure, is_generator, globals->cls == dict_cls ? static_cast<BoxedDict*>(globals) : NULL,
-                         u.il);
+
+    Box* passed_globals = NULL;
+    if (!getCF()->clfunc->source->scoping->areGlobalsFromModule())
+        passed_globals = globals;
+    return boxCLFunction(cl, closure, is_generator, passed_globals, u.il);
 }
 
 Value ASTInterpreter::visit_makeFunction(AST_MakeFunction* mkfn) {
@@ -804,9 +807,11 @@ Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) {
 
     BoxedClosure* closure = scope_info->takesClosure() ? created_closure : 0;
     CLFunction* cl = wrapFunction(node, nullptr, node->body, source_info);
-    Box* attrDict = runtimeCall(
-        boxCLFunction(cl, closure, false, globals->cls == dict_cls ? static_cast<BoxedDict*>(globals) : NULL, {}),
-        ArgPassSpec(0), 0, 0, 0, 0, 0);
+
+    Box* passed_globals = NULL;
+    if (!getCF()->clfunc->source->scoping->areGlobalsFromModule())
+        passed_globals = globals;
+    Box* attrDict = runtimeCall(boxCLFunction(cl, closure, false, passed_globals, {}), ArgPassSpec(0), 0, 0, 0, 0, 0);
 
     Box* classobj = createUserClass(&node->name.str(), basesTuple, attrDict);
 
@@ -1202,8 +1207,9 @@ Value ASTInterpreter::visit_attribute(AST_Attribute* node) {
 
 const void* interpreter_instr_addr = (void*)&ASTInterpreter::execute;
 
-Box* astInterpretFunction(CompiledFunction* cf, int nargs, Box* closure, Box* generator, BoxedDict* globals, Box* arg1,
+Box* astInterpretFunction(CompiledFunction* cf, int nargs, Box* closure, Box* generator, Box* globals, Box* arg1,
                           Box* arg2, Box* arg3, Box** args) {
+    assert((!globals) == cf->clfunc->source->scoping->areGlobalsFromModule());
     bool can_reopt = ENABLE_REOPT && !FORCE_INTERPRETER && (globals == NULL);
     if (unlikely(can_reopt && cf->times_called > REOPT_THRESHOLD_INTERPRETER)) {
         assert(!globals);
@@ -1240,7 +1246,7 @@ Box* astInterpretFunction(CompiledFunction* cf, int nargs, Box* closure, Box* ge
     return v.o ? v.o : None;
 }
 
-Box* astInterpretFunctionEval(CompiledFunction* cf, BoxedDict* globals, Box* boxedLocals) {
+Box* astInterpretFunctionEval(CompiledFunction* cf, Box* globals, Box* boxedLocals) {
     ++cf->times_called;
 
     ASTInterpreter interpreter(cf);
@@ -1249,13 +1255,10 @@ Box* astInterpretFunctionEval(CompiledFunction* cf, BoxedDict* globals, Box* box
 
     ScopeInfo* scope_info = cf->clfunc->source->getScopeInfo();
     SourceInfo* source_info = cf->clfunc->source;
-    if (cf->clfunc->source->scoping->areGlobalsFromModule()) {
-        assert(!globals);
-        interpreter.setGlobals(source_info->parent_module);
-    } else {
-        assert(globals);
-        interpreter.setGlobals(globals);
-    }
+
+    assert(!cf->clfunc->source->scoping->areGlobalsFromModule());
+    assert(globals);
+    interpreter.setGlobals(globals);
 
     Value v = ASTInterpreter::execute(interpreter);
 

src/codegen/ast_interpreter.h

@@ -33,9 +33,9 @@ struct LineInfo;
 
 extern const void* interpreter_instr_addr;
 
-Box* astInterpretFunction(CompiledFunction* f, int nargs, Box* closure, Box* generator, BoxedDict* globals, Box* arg1,
+Box* astInterpretFunction(CompiledFunction* f, int nargs, Box* closure, Box* generator, Box* globals, Box* arg1,
                           Box* arg2, Box* arg3, Box** args);
-Box* astInterpretFunctionEval(CompiledFunction* cf, BoxedDict* globals, Box* boxedLocals);
+Box* astInterpretFunctionEval(CompiledFunction* cf, Box* globals, Box* boxedLocals);
 Box* astInterpretFrom(CompiledFunction* cf, AST_expr* after_expr, AST_stmt* enclosing_stmt, Box* expr_val,
                       FrameStackState frame_state);
 

src/codegen/compvars.cpp

@@ -721,7 +721,7 @@ ConcreteCompilerVariable* UnknownType::hasnext(IREmitter& emitter, const OpInfo&
 }
 
 CompilerVariable* makeFunction(IREmitter& emitter, CLFunction* f, CompilerVariable* closure, bool isGenerator,
-                               BoxedDict* globals, const std::vector<ConcreteCompilerVariable*>& defaults) {
+                               Box* globals, const std::vector<ConcreteCompilerVariable*>& defaults) {
     // Unlike the CLFunction*, which can be shared between recompilations, the Box* around it
     // should be created anew every time the functiondef is encountered
 
@@ -752,7 +752,7 @@ CompilerVariable* makeFunction(IREmitter& emitter, CLFunction* f, CompilerVariab
     llvm::Value* isGenerator_v = llvm::ConstantInt::get(g.i1, isGenerator, false);
 
     assert(globals == NULL);
-    llvm::Value* globals_v = getNullPtr(g.llvm_dict_type_ptr);
+    llvm::Value* globals_v = getNullPtr(g.llvm_value_type_ptr);
 
     // We know this function call can't throw, so it's safe to use emitter.getBuilder()->CreateCall() rather than
     // emitter.createCall().

src/codegen/compvars.h

@@ -399,7 +399,7 @@ ConcreteCompilerVariable* makePureImaginary(IREmitter& emitter, double imag);
 CompilerVariable* makeStr(const std::string*);
 CompilerVariable* makeUnicode(IREmitter& emitter, const std::string*);
 CompilerVariable* makeFunction(IREmitter& emitter, CLFunction*, CompilerVariable* closure, bool isGenerator,
-                               BoxedDict* globals, const std::vector<ConcreteCompilerVariable*>& defaults);
+                               Box* globals, const std::vector<ConcreteCompilerVariable*>& defaults);
 ConcreteCompilerVariable* undefVariable();
 CompilerVariable* makeTuple(const std::vector<CompilerVariable*>& elts);
 

src/codegen/irgen/hooks.cpp

@@ -332,35 +332,37 @@ void compileAndRunModule(AST_Module* m, BoxedModule* bm) {
         ((void (*)())cf->code)();
 }
 
-template <typename AST_Type>
-Box* evalOrExec(AST_Type* source, std::vector<AST_stmt*>& body, BoxedModule* bm, BoxedDict* globals, Box* boxedLocals) {
-    CompiledFunction* cf;
+Box* evalOrExec(CLFunction* cl, Box* globals, Box* boxedLocals) {
+    RELEASE_ASSERT(!cl->source->scoping->areGlobalsFromModule(), "");
 
-    assert(!globals || globals->cls == dict_cls);
+    assert(globals && (globals->cls == module_cls || globals->cls == dict_cls));
 
-    { // scope for limiting the locked region:
-        LOCK_REGION(codegen_rwlock.asWrite());
+    // TODO Right now we only support going into an exec or eval through the
+    // intepretter, since the interpretter has a special function which lets
+    // us set the locals object. We should probably support it for optimized
+    // code as well, so we could use initialEffort() here instead of hard-coding
+    // INTERPRETED. This could actually be useful if we actually cache the parse
+    // results (since sometimes eval or exec might be called on constant strings).
+    EffortLevel effort = EffortLevel::INTERPRETED;
 
-        Timer _t("for evalOrExec()");
+    CompiledFunction* cf = compileFunction(cl, new FunctionSpecialization(VOID), effort, NULL);
+    assert(cf->clfunc->versions.size());
 
-        ScopingAnalysis* scoping = new ScopingAnalysis(source, globals == NULL);
+    return astInterpretFunctionEval(cf, globals, boxedLocals);
+}
 
-        SourceInfo* si = new SourceInfo(bm, scoping, source, body);
-        CLFunction* cl_f = new CLFunction(0, 0, false, false, si);
+template <typename AST_Type>
+CLFunction* compileForEvalOrExec(AST_Type* source, std::vector<AST_stmt*>& body, BoxedModule* bm) {
+    LOCK_REGION(codegen_rwlock.asWrite());
 
-        // TODO Right now we only support going into an exec or eval through the
-        // intepretter, since the interpretter has a special function which lets
-        // us set the locals object. We should probably support it for optimized
-        // code as well, so we could use initialEffort() here instead of hard-coding
-        // INTERPRETED. This could actually be useful if we actually cache the parse
-        // results (since sometimes eval or exec might be called on constant strings).
-        EffortLevel effort = EffortLevel::INTERPRETED;
+    Timer _t("for evalOrExec()");
 
-        cf = compileFunction(cl_f, new FunctionSpecialization(VOID), effort, NULL);
-        assert(cf->clfunc->versions.size());
-    }
+    ScopingAnalysis* scoping = new ScopingAnalysis(source, false);
 
-    return astInterpretFunctionEval(cf, globals, boxedLocals);
+    SourceInfo* si = new SourceInfo(bm, scoping, source, body);
+    CLFunction* cl_f = new CLFunction(0, 0, false, false, si);
+
+    return cl_f;
 }
 
 // Main entrypoints for eval and exec.
@@ -369,11 +371,8 @@ Box* eval(Box* boxedCode) {
     BoxedModule* module = getCurrentModule();
     Box* globals = getGlobals();
 
-    if (globals == module)
-        globals = NULL;
-
     if (globals && globals->cls == attrwrapper_cls && unwrapAttrWrapper(globals) == module)
-        globals = NULL;
+        globals = module;
 
     if (boxedCode->cls == unicode_cls) {
         boxedCode = PyUnicode_AsUTF8String(boxedCode);
@@ -408,9 +407,10 @@ Box* eval(Box* boxedCode) {
     stmt->value = parsedExpr->body;
     std::vector<AST_stmt*> body = { stmt };
 
-    assert(!globals || globals->cls == dict_cls);
+    assert(globals && (globals->cls == module_cls || globals->cls == dict_cls));
 
-    return evalOrExec<AST_Expression>(parsedExpr, body, module, static_cast<BoxedDict*>(globals), boxedLocals);
+    CLFunction* cl = compileForEvalOrExec(parsedExpr, body, module);
+    return evalOrExec(cl, globals, boxedLocals);
 }
 
 Box* exec(Box* boxedCode, Box* globals, Box* locals) {
@@ -447,18 +447,18 @@ Box* exec(Box* boxedCode, Box* globals, Box* locals) {
 
     BoxedModule* module = getCurrentModule();
 
-    if (globals == module)
-        globals = NULL;
-
     if (globals && globals->cls == attrwrapper_cls && unwrapAttrWrapper(globals) == module)
-        globals = NULL;
+        globals = module;
 
-    ASSERT(!globals || globals->cls == dict_cls, "%s", globals->cls->tp_name);
+    assert(globals && (globals->cls == module_cls || globals->cls == dict_cls));
 
     if (globals) {
         // From CPython (they set it to be f->f_builtins):
-        if (PyDict_GetItemString(globals, "__builtins__") == NULL)
-            PyDict_SetItemString(globals, "__builtins__", builtins_module);
+        Box* globals_dict = globals;
+        if (globals->cls == module_cls)
+            globals_dict = makeAttrWrapper(globals);
+        if (PyDict_GetItemString(globals_dict, "__builtins__") == NULL)
+            PyDict_SetItemString(globals_dict, "__builtins__", builtins_module);
     }
 
     if (boxedCode->cls == unicode_cls) {
@@ -475,7 +475,8 @@ Box* exec(Box* boxedCode, Box* globals, Box* locals) {
     AST_Suite* parsedSuite = new AST_Suite(std::move(parsedModule->interned_strings));
     parsedSuite->body = parsedModule->body;
 
-    return evalOrExec<AST_Suite>(parsedSuite, parsedSuite->body, module, static_cast<BoxedDict*>(globals), locals);
+    CLFunction* cl = compileForEvalOrExec(parsedSuite, parsedSuite->body, module);
+    return evalOrExec(cl, globals, locals);
 }
 
 // If a function version keeps failing its speculations, kill it (remove it

src/runtime/objmodel.cpp

@@ -2975,7 +2975,7 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
 }
 
 Box* callCLFunc(CLFunction* f, CallRewriteArgs* rewrite_args, int num_output_args, BoxedClosure* closure,
-                BoxedGenerator* generator, BoxedDict* globals, Box* oarg1, Box* oarg2, Box* oarg3, Box** oargs) {
+                BoxedGenerator* generator, Box* globals, Box* oarg1, Box* oarg2, Box* oarg3, Box** oargs) {
     CompiledFunction* chosen_cf = pickVersion(f, num_output_args, oarg1, oarg2, oarg3, oargs);
 
     assert(chosen_cf->is_interpreted == (chosen_cf->code == NULL));

src/runtime/objmodel.h

@@ -150,7 +150,7 @@ bool isUserDefined(BoxedClass* cls);
 Box* processDescriptor(Box* obj, Box* inst, Box* owner);
 
 Box* callCLFunc(CLFunction* f, CallRewriteArgs* rewrite_args, int num_output_args, BoxedClosure* closure,
-                BoxedGenerator* generator, BoxedDict* globals, Box* oarg1, Box* oarg2, Box* oarg3, Box** oargs);
+                BoxedGenerator* generator, Box* globals, Box* oarg1, Box* oarg2, Box* oarg3, Box** oargs);
 
 static const char* objectNewParameterTypeErrorMsg() {
     if (PYTHON_VERSION_HEX >= version_hex(2, 7, 4)) {

src/runtime/types.cpp

@@ -311,9 +311,10 @@ BoxedFunction::BoxedFunction(CLFunction* f) : BoxedFunction(f, {}) {
 }
 
 BoxedFunction::BoxedFunction(CLFunction* f, std::initializer_list<Box*> defaults, BoxedClosure* closure,
-                             bool isGenerator, BoxedDict* globals)
+                             bool isGenerator, Box* globals)
     : BoxedFunctionBase(f, defaults, closure, isGenerator) {
 
+    assert((!globals) == (!f->source || f->source->scoping->areGlobalsFromModule()));
     this->globals = globals;
 
     // TODO eventually we want this to assert(f->source), I think, but there are still
@@ -418,12 +419,10 @@ extern "C" void moduleGCHandler(GCVisitor* v, Box* b) {
 // This mustn't throw; our IR generator generates calls to it without "invoke" even when there are exception handlers /
 // finally-blocks in scope.
 // TODO: should we use C++11 `noexcept' here?
-extern "C" Box* boxCLFunction(CLFunction* f, BoxedClosure* closure, bool isGenerator, BoxedDict* globals,
+extern "C" Box* boxCLFunction(CLFunction* f, BoxedClosure* closure, bool isGenerator, Box* globals,
                               std::initializer_list<Box*> defaults) {
     if (closure)
         assert(closure->cls == closure_cls);
-    if (globals)
-        assert(globals->cls == dict_cls);
 
     return new BoxedFunction(f, defaults, closure, isGenerator, globals);
 }

src/runtime/types.h

@@ -128,7 +128,7 @@ char* getWriteableStringContents(BoxedString* s);
 
 extern "C" void listAppendInternal(Box* self, Box* v);
 extern "C" void listAppendArrayInternal(Box* self, Box** v, int nelts);
-extern "C" Box* boxCLFunction(CLFunction* f, BoxedClosure* closure, bool isGenerator, BoxedDict* globals,
+extern "C" Box* boxCLFunction(CLFunction* f, BoxedClosure* closure, bool isGenerator, Box* globals,
                               std::initializer_list<Box*> defaults);
 extern "C" CLFunction* unboxCLFunction(Box* b);
 extern "C" Box* createUserClass(const std::string* name, Box* base, Box* attr_dict);
@@ -591,7 +591,7 @@ public:
     // garbage values when the GC is run (BoxedFunctionBase's constructor might call the GC).
     // So ick... needs to be fixed.
     BoxedClosure* closure;
-    BoxedDict* globals;
+    Box* globals;
 
     bool isGenerator;
     int ndefaults;
@@ -615,7 +615,7 @@ public:
 
     BoxedFunction(CLFunction* f);
     BoxedFunction(CLFunction* f, std::initializer_list<Box*> defaults, BoxedClosure* closure = NULL,
-                  bool isGenerator = false, BoxedDict* globals = NULL);
+                  bool isGenerator = false, Box* globals = NULL);
 
     DEFAULT_CLASS(function_cls);
 };