Fix a bug in expression evaluation order

Also, give up on maintaining 100% error message compatibility
with CPython.  There are some places where it's kind of silly
to do this since the messages seem based on internal implementation
details; namely, whether to output at the global scope:
NameError: global name 'x' is not defined
vs
NameError: name 'x' is not defined
This depends on whether any function has been defined that has
a "global x" declaration.

Instead, make the tester a little bit smarter to know that certain
classes of error messages should be treated as equivalent.

src/codegen/irgen/irgenerator.cpp

@@ -756,7 +756,6 @@ private:
                 // Method 1: calls into the runtime getGlobal(), which handles things like falling back to builtins
                 // or raising the correct error message.
                 bool do_patchpoint = ENABLE_ICGETGLOBALS && (irstate->getEffortLevel() != EffortLevel::INTERPRETED);
-                bool from_global = irstate->getSourceInfo()->ast->type == AST_TYPE::Module;
                 if (do_patchpoint) {
                     PatchpointSetupInfo* pp = patchpoints::createGetGlobalPatchpoint(
                         emitter.currentFunction(), getOpInfoForNode(node, exc_info).getTypeRecorder());
@@ -765,18 +764,17 @@ private:
                     llvm_args.push_back(
                         embedConstantPtr(irstate->getSourceInfo()->parent_module, g.llvm_module_type_ptr));
                     llvm_args.push_back(embedConstantPtr(&node->id, g.llvm_str_type_ptr));
-                    llvm_args.push_back(getConstantInt(from_global, g.i1));
 
                     llvm::Value* uncasted
                         = emitter.createPatchpoint(pp, (void*)pyston::getGlobal, llvm_args, exc_info).getInstruction();
                     llvm::Value* r = emitter.getBuilder()->CreateIntToPtr(uncasted, g.llvm_value_type_ptr);
                     return new ConcreteCompilerVariable(UNKNOWN, r, true);
                 } else {
-                    llvm::Value* r = emitter.createCall3(exc_info, g.funcs.getGlobal,
-                                                         embedConstantPtr(irstate->getSourceInfo()->parent_module,
-                                                                          g.llvm_module_type_ptr),
-                                                         embedConstantPtr(&node->id, g.llvm_str_type_ptr),
-                                                         getConstantInt(from_global, g.i1)).getInstruction();
+                    llvm::Value* r
+                        = emitter.createCall2(
+                                      exc_info, g.funcs.getGlobal,
+                                      embedConstantPtr(irstate->getSourceInfo()->parent_module, g.llvm_module_type_ptr),
+                                      embedConstantPtr(&node->id, g.llvm_str_type_ptr)).getInstruction();
                     return new ConcreteCompilerVariable(UNKNOWN, r, true);
                 }
             } else {

src/core/cfg.cpp

@@ -661,7 +661,8 @@ private:
                 rtn = remapListComp(ast_cast<AST_ListComp>(node));
                 break;
             case AST_TYPE::Name:
-                return node;
+                rtn = node;
+                break;
             case AST_TYPE::Num:
                 return node;
             case AST_TYPE::Repr:
@@ -685,7 +686,7 @@ private:
                 RELEASE_ASSERT(0, "%d", node->type);
         }
 
-        if (wrap_with_assign && rtn->type != AST_TYPE::Name) {
+        if (wrap_with_assign && (rtn->type != AST_TYPE::Name || ast_cast<AST_Name>(rtn)->id[0] != '#')) {
             std::string name = nodeName(node);
             push_back(makeAssign(name, rtn));
             return makeName(name, AST_TYPE::Load);

src/runtime/objmodel.cpp

@@ -2362,7 +2362,7 @@ Box* typeNew(Box* cls, Box* obj) {
     return rtn;
 }
 
-extern "C" Box* getGlobal(BoxedModule* m, std::string* name, bool from_global) {
+extern "C" Box* getGlobal(BoxedModule* m, std::string* name) {
     static StatCounter slowpath_getglobal("slowpath_getglobal");
     slowpath_getglobal.log();
     static StatCounter nopatch_getglobal("nopatch_getglobal");
@@ -2429,10 +2429,7 @@ extern "C" Box* getGlobal(BoxedModule* m, std::string* name, bool from_global) {
             return rtn;
     }
 
-    if (from_global)
-        raiseExcHelper(NameError, "name '%s' is not defined", name->c_str());
-    else
-        raiseExcHelper(NameError, "global name '%s' is not defined", name->c_str());
+    raiseExcHelper(NameError, "global name '%s' is not defined", name->c_str());
 }
 
 // TODO I feel like importing should go somewhere else; it's more closely tied to codegen

src/runtime/objmodel.h

@@ -57,7 +57,7 @@ extern "C" void dump(Box* obj);
 extern "C" i64 unboxedLen(Box* obj);
 extern "C" Box* binop(Box* lhs, Box* rhs, int op_type);
 extern "C" Box* augbinop(Box* lhs, Box* rhs, int op_type);
-extern "C" Box* getGlobal(BoxedModule* m, std::string* name, bool from_global);
+extern "C" Box* getGlobal(BoxedModule* m, std::string* name);
 extern "C" Box* getitem(Box* value, Box* slice);
 extern "C" void setitem(Box* target, Box* slice, Box* value);
 extern "C" Box* getclsattr(Box* obj, const char* attr);

test/tests/expr_evaluation_order.py

+# The function should get evaluated before the arguments
+# This is especially tricky if the evaluation of the function
+# expression can change the behavior of the argument expressions.
+
+def f1():
+    global g1
+    def g1(x):
+        return x
+    def inner(x):
+        return x
+    return inner
+
+print f1()(g1(1))
+
+def f2(y):
+    global g2
+    def g2(x):
+        return x
+    return y
+
+print g2(f2(2))

tools/tester.py

@@ -72,12 +72,32 @@ def get_expected_output(fn):
     out, err = p.communicate()
     code = p.wait()
 
-    r = code, out, err.strip().split('\n')[-1]
+    r = code, out, err
     assert code >= 0, "CPython exited with an unexpected exit code: %d" % (code,)
 
     cPickle.dump(r, open(cache_fn, 'w'))
     return r
 
+def canonicalize_stderr(stderr):
+    """
+    For a while we were trying to maintain *exact* stderr compatibility with CPython,
+    at least for the last line of the stderr.
+
+    It was starting to get silly to do this, so instead apply some "canonicalizations"
+    to map certain groups of error messages together.
+    """
+
+    stderr = stderr.strip().split('\n')[-1]
+
+    substitutions = [
+            ("NameError: global name '", "NameError: name '"),
+            ]
+
+    for pattern, subst_with in substitutions:
+        stderr = stderr.replace(pattern, subst_with)
+
+    return stderr
+
 failed = []
 def run_test(fn, check_stats, run_memcheck):
     r = fn.rjust(FN_JUST_SIZE)
@@ -107,9 +127,8 @@ def run_test(fn, check_stats, run_memcheck):
     run_args = [os.path.abspath(IMAGE)] + jit_args + [fn]
     start = time.time()
     p = subprocess.Popen(run_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=open("/dev/null"), preexec_fn=set_ulimits)
-    out, err = p.communicate()
-    full_err = err
-    err = err.strip().split('\n')[-1]
+    out, stderr = p.communicate()
+    last_stderr_line = stderr.strip().split('\n')[-1]
 
     code = p.wait()
     elapsed = time.time() - start
@@ -128,6 +147,8 @@ def run_test(fn, check_stats, run_memcheck):
 
         if code == 0:
             err = "(Unexpected success)"
+        else:
+            err = last_stderr_line
 
         if code == -signal.SIGALRM:
             msg = "Timed out"
@@ -146,7 +167,7 @@ def run_test(fn, check_stats, run_memcheck):
                 failed.append(fn)
                 return r
             else:
-                raise Exception("%s\n%s\n%s" % (msg, err, full_err))
+                raise Exception("%s\n%s\n%s" % (msg, err, stderr))
     elif out != expected_out:
         if expected == "fail":
             r += "    Expected failure (bad output)"
@@ -164,13 +185,13 @@ def run_test(fn, check_stats, run_memcheck):
             diff = p.stdout.read()
             assert p.wait() in (0, 1)
             raise Exception("Failed on %s:\n%s" % (fn, diff))
-    elif not TEST_PYPY and err != expected_err:
+    elif not TEST_PYPY and canonicalize_stderr(stderr) != canonicalize_stderr(expected_err):
         if KEEP_GOING:
             r += "    \033[31mFAILED\033[0m (bad stderr)"
             failed.append(fn)
             return r
         else:
-            raise Exception((err, expected_err))
+            raise Exception((canonicalize_stderr(stderr), canonicalize_stderr(expected_err)))
     elif expected == "fail":
         if KEEP_GOING:
             r += "    \033[31mFAILED\033[0m (unexpected success)"