add warning for exec scope issue

Author: Eddy Xu

Lib/test/test_py3kwarn.py

@@ -1,6 +1,7 @@
 import unittest
 import sys
 from test.test_support import check_py3k_warnings, CleanImport, run_unittest
+from test.script_helper import assert_python_ok
 import warnings
 import base64
 from test import test_support
@@ -438,43 +439,16 @@ def assertExecLocalWritebackWarning(self, recorder, local_name):
         recorder.reset()
 
     def test_exec_local_writeback_warning(self):
-        def f_exec(code):
-            b = 42
-            exec code
-            return b
-
-        with check_py3k_warnings() as w:
-            f_exec("b = 99")
-            self.assertExecLocalWritebackWarning(w, "b")
-
-        def f_exec_no_write(code):
-            b = 42
-            exec code
-            return b
-
-        with check_py3k_warnings() as w:
-            f_exec_no_write("print b")
-            self.assertEqual(len(w.warnings), 0)
-
-        def f_exec_overwrite(code):
-            b = 42
-            exec code
-            b = 7
-            return b
-
-        with check_py3k_warnings() as w:
-            f_exec_overwrite("b = 99")
-            self.assertEqual(len(w.warnings), 0)
-
-        def f_exec_explicit(code):
-            b = 42
-            ns = {'b': b}
-            exec code in globals(), ns
-            return b
-
-        with check_py3k_warnings() as w:
-            f_exec_explicit("b = 99")
-            self.assertEqual(len(w.warnings), 0)
+        rc, out, err = assert_python_ok(
+            "-3",
+            "-c",
+            "def f(code):\n"
+            "    b = 42\n"
+            "    exec code\n"
+            "    return b\n"
+            "f('b = 99')\n")
+        self.assertEqual(rc, 0)
+        self.assertIn("exec() modified local 'b' which is read later", err)
 
 
 class TestStdlibRemovals(unittest.TestCase):

Python/ceval.c

@@ -81,23 +81,28 @@ static void
 _clear_exec_local_writeback_for_frame(PyFrameObject *f)
 {
     PyObject *frame_key;
+    PyObject *exc_type, *exc_value, *exc_tb;
 
     if (exec_local_writeback_map == NULL) {
         return;
     }
+    PyErr_Fetch(&exc_type, &exc_value, &exc_tb);
     frame_key = PyLong_FromVoidPtr(f);
     if (frame_key == NULL) {
         PyErr_Clear();
+        PyErr_Restore(exc_type, exc_value, exc_tb);
         return;
     }
-    if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
+    if (PyDict_GetItem(exec_local_writeback_map, frame_key) != NULL &&
+            PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
         PyErr_Clear();
     }
     Py_DECREF(frame_key);
+    PyErr_Restore(exc_type, exc_value, exc_tb);
 }
 
 static int
-_warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode)
+_warn_exec_local_writeback(PyFrameObject *f, int oparg)
 {
     PyObject *frame_key = NULL;
     PyObject *frame_dict = NULL;
@@ -106,11 +111,7 @@ _warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode)
     PyObject *msg = NULL;
     PyObject *name_obj = NULL;
     const char *local_name = NULL;
-    const char *func_name = NULL;
-    int exec_lineno = 0;
-    int exec_offset = 0;
     int read_lineno = 0;
-    int read_offset = f->f_lasti;
     int warn_result;
 
     if (exec_local_writeback_map == NULL) {
@@ -138,37 +139,18 @@ _warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode)
         Py_DECREF(frame_key);
         return 0;
     }
-    if (PyTuple_Check(entry) && PyTuple_GET_SIZE(entry) == 2) {
-        exec_lineno = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 0));
-        exec_offset = (int)PyInt_AsLong(PyTuple_GET_ITEM(entry, 1));
-    }
-    if (exec_lineno < 0) {
-        exec_lineno = 0;
-    }
     read_lineno = PyCode_Addr2Line(f->f_code, f->f_lasti);
     name_obj = PyTuple_GetItem(f->f_code->co_varnames, oparg);
     if (name_obj != NULL) {
         local_name = PyString_AsString(name_obj);
     }
-    if (local_name == NULL) {
+    else if (local_name == NULL) {
         local_name = "<local>";
     }
-    if (f->f_code->co_name != NULL) {
-        func_name = PyString_AsString(f->f_code->co_name);
-    }
-    if (func_name == NULL) {
-        func_name = "<function>";
-    }
     msg = PyString_FromFormat(
         "exec() modified local '%s' which is read later in the enclosing function; "
-        "in 3.x exec() does not reliably write back to function locals without an explicit locals mapping"
-        "\nPYGRATE_META: {\"warning_type\":\"EXEC_LOCAL_WRITEBACK_WARNING\","
-        "\"scope_kind\":\"function\",\"has_explicit_globals\":false,"
-        "\"has_explicit_locals\":false,\"local_name\":\"%s\","
-        "\"exec_lineno\":%d,\"read_lineno\":%d,\"read_opcode\":%d,"
-        "\"exec_offset\":%d,\"read_offset\":%d,\"function_name\":\"%s\"}",
-        local_name, local_name, exec_lineno, read_lineno, opcode,
-        exec_offset, read_offset, func_name);
+        "in 3.x exec() does not reliably write back to function locals without an explicit locals mapping",
+        local_name);
     if (msg == NULL) {
         Py_DECREF(idx);
         Py_DECREF(frame_key);
@@ -197,10 +179,9 @@ _warn_exec_local_writeback(PyFrameObject *f, int oparg, int opcode)
     }
     Py_DECREF(idx);
 
-    if (PyDict_Size(frame_dict) <= 0) {
-        if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
-            PyErr_Clear();
-        }
+    if (PyDict_Size(frame_dict) <= 0 &&
+            PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
+        PyErr_Clear();
     }
     Py_DECREF(frame_key);
     return 0;
@@ -212,36 +193,42 @@ _clear_exec_local_writeback_for_local(PyFrameObject *f, int oparg)
     PyObject *frame_key = NULL;
     PyObject *frame_dict = NULL;
     PyObject *idx = NULL;
+    PyObject *exc_type, *exc_value, *exc_tb;
 
     if (exec_local_writeback_map == NULL) {
         return;
     }
+    PyErr_Fetch(&exc_type, &exc_value, &exc_tb);
     frame_key = PyLong_FromVoidPtr(f);
     if (frame_key == NULL) {
         PyErr_Clear();
+        PyErr_Restore(exc_type, exc_value, exc_tb);
         return;
     }
     frame_dict = PyDict_GetItem(exec_local_writeback_map, frame_key);
     if (frame_dict == NULL) {
         Py_DECREF(frame_key);
+        PyErr_Restore(exc_type, exc_value, exc_tb);
         return;
     }
     idx = PyInt_FromLong(oparg);
     if (idx == NULL) {
         Py_DECREF(frame_key);
         PyErr_Clear();
+        PyErr_Restore(exc_type, exc_value, exc_tb);
         return;
     }
-    if (PyDict_DelItem(frame_dict, idx) < 0) {
+    if (PyDict_GetItem(frame_dict, idx) != NULL &&
+            PyDict_DelItem(frame_dict, idx) < 0) {
         PyErr_Clear();
     }
     Py_DECREF(idx);
-    if (PyDict_Size(frame_dict) <= 0) {
-        if (PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
-            PyErr_Clear();
-        }
+    if (PyDict_Size(frame_dict) <= 0 &&
+            PyDict_DelItem(exec_local_writeback_map, frame_key) < 0) {
+        PyErr_Clear();
     }
     Py_DECREF(frame_key);
+    PyErr_Restore(exc_type, exc_value, exc_tb);
 }
 
 #ifndef WITH_TSC
@@ -1451,11 +1438,10 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
         {
             x = GETLOCAL(oparg);
             if (x != NULL) {
-                if (Py_Py3kWarningFlag) {
-                    if (_warn_exec_local_writeback(f, oparg, opcode) < 0) {
-                        err = -1;
-                        break;
-                    }
+                if (Py_Py3kWarningFlag &&
+                        _warn_exec_local_writeback(f, oparg) < 0) {
+                    err = -1;
+                    break;
                 }
                 Py_INCREF(x);
                 PUSH(x);
@@ -5277,7 +5263,7 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals,
     int n;
     PyObject *v;
     int plain = 0;
-    int track_locals = 0;
+    int track_exec_writeback = 0;
     int exec_lineno = 0;
     int exec_offset = 0;
     int nlocals = 0;
@@ -5329,6 +5315,8 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals,
     if (PyDict_GetItemString(globals, "__builtins__") == NULL)
         PyDict_SetItemString(globals, "__builtins__", f->f_builtins);
 
+    /* Snapshot fast locals for plain exec so later LOAD_FAST warns only
+       about bindings that exec actually wrote back. */
     if (plain && Py_Py3kWarningFlag &&
         (f->f_code->co_flags & CO_NEWLOCALS) &&
         f->f_code->co_nlocals > 0 &&
@@ -5340,7 +5328,7 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals,
                 before[i] = f->f_localsplus[i];
                 Py_XINCREF(before[i]);
             }
-            track_locals = 1;
+            track_exec_writeback = 1;
             exec_offset = f->f_lasti;
             exec_lineno = PyCode_Addr2Line(f->f_code, exec_offset);
         }
@@ -5392,7 +5380,7 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals,
     }
     if (plain)
         PyFrame_LocalsToFast(f, 0);
-    if (track_locals && v != NULL) {
+    if (track_exec_writeback && v != NULL) {
         PyObject *frame_key = NULL;
         PyObject *frame_dict = NULL;
         PyObject *map = NULL;
@@ -5437,11 +5425,8 @@ exec_statement(PyFrameObject *f, PyObject *prog, PyObject *globals,
             if (frame_dict != NULL) {
                 PyObject *idx = PyInt_FromLong(i);
                 PyObject *val = Py_BuildValue("ii", exec_lineno, exec_offset);
-                if (idx != NULL && val != NULL) {
-                    if (PyDict_SetItem(frame_dict, idx, val) < 0) {
-                        PyErr_Clear();
-                    }
-                } else {
+                if (idx == NULL || val == NULL ||
+                        PyDict_SetItem(frame_dict, idx, val) < 0) {
                     PyErr_Clear();
                 }
                 Py_XDECREF(idx);