Do not copy free variables to locals in class namespaces.

Fixes bug 1569356, but at the cost of a minor incompatibility in
locals().  Add test that verifies that the class namespace is not
polluted.  Also clarify the behavior in the library docs.

Along the way, cleaned up the dict_to_map and map_to_dict
implementations and added some comments that explain what they do.

Doc/lib/libfuncs.tex

@@ -607,6 +607,11 @@ class C:
   \warning{The contents of this dictionary should not be modified;
   changes may not affect the values of local variables used by the
   interpreter.}
+
+  Free variables are returned by \var{locals} when it is called in
+  a function block.  Modifications of free variables may not affect
+  the values used by the interpreter.  Free variables are not
+  returned in class blocks.
 \end{funcdesc}
 
 \begin{funcdesc}{long}{\optional{x\optional{, radix}}}

Lib/test/test_scope.py

@@ -486,6 +486,39 @@ self.assert_(X.passed)
         del d['h']
         self.assertEqual(d, {'x': 2, 'y': 7, 'w': 6})
 
+    def testLocalsClass(self):
+        # This test verifies that calling locals() does not pollute
+        # the local namespace of the class with free variables.  Old
+        # versions of Python had a bug, where a free variable being
+        # passed through a class namespace would be inserted into
+        # locals() by locals() or exec or a trace function.
+        #
+        # The real bug lies in frame code that copies variables
+        # between fast locals and the locals dict, e.g. when executing
+        # a trace function.
+
+        def f(x):
+            class C:
+                x = 12
+                def m(self):
+                    return x
+                locals()
+            return C
+
+        self.assertEqual(f(1).x, 12)
+
+        def f(x):
+            class C:
+                y = x
+                def m(self):
+                    return x
+                z = list(locals())
+            return C
+
+        varnames = f(1).z
+        self.assert_("x" not in varnames)
+        self.assert_("y" in varnames)
+
     def testBoundAndFree(self):
         # var is bound and free in class
 

Objects/frameobject.c

@@ -701,18 +701,38 @@ PyFrame_BlockPop(PyFrameObject *f)
 	return b;
 }
 
-/* Convert between "fast" version of locals and dictionary version */
+/* Convert between "fast" version of locals and dictionary version.
+   
+   map and values are input arguments.  map is a tuple of strings.
+   values is an array of PyObject*.  At index i, map[i] is the name of
+   the variable with value values[i].  The function copies the first
+   nmap variable from map/values into dict.  If values[i] is NULL,
+   the variable is deleted from dict.
+
+   If deref is true, then the values being copied are cell variables
+   and the value is extracted from the cell variable before being put
+   in dict.
+
+   Exceptions raised while modifying the dict are silently ignored,
+   because there is no good way to report them.
+ */
 
 static void
 map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
-	    Py_ssize_t deref)
+	    int deref)
 {
 	Py_ssize_t j;
+        assert(PyTuple_Check(map));
+        assert(PyDict_Check(dict));
+        assert(PyTuple_Size(map) > nmap);
 	for (j = nmap; --j >= 0; ) {
 		PyObject *key = PyTuple_GET_ITEM(map, j);
 		PyObject *value = values[j];
-		if (deref)
+                assert(PyString_Check(key));
+		if (deref) {
+                        assert(PyCell_Check(value));
 			value = PyCell_GET(value);
+                }
 		if (value == NULL) {
 			if (PyObject_DelItem(dict, key) != 0)
 				PyErr_Clear();
@@ -724,29 +744,55 @@ map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
 	}
 }
 
+/* Copy values from the "locals" dict into the fast locals.
+
+   dict is an input argument containing string keys representing
+   variables names and arbitrary PyObject* as values.
+
+   map and values are input arguments.  map is a tuple of strings.
+   values is an array of PyObject*.  At index i, map[i] is the name of
+   the variable with value values[i].  The function copies the first
+   nmap variable from map/values into dict.  If values[i] is NULL,
+   the variable is deleted from dict.
+
+   If deref is true, then the values being copied are cell variables
+   and the value is extracted from the cell variable before being put
+   in dict.  If clear is true, then variables in map but not in dict
+   are set to NULL in map; if clear is false, variables missing in
+   dict are ignored.
+
+   Exceptions raised while modifying the dict are silently ignored,
+   because there is no good way to report them.
+*/
+
 static void
 dict_to_map(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
-	    Py_ssize_t deref, int clear)
+	    int deref, int clear)
 {
 	Py_ssize_t j;
+        assert(PyTuple_Check(map));
+        assert(PyDict_Check(dict));
+        assert(PyTuple_Size(map) > nmap);
 	for (j = nmap; --j >= 0; ) {
 		PyObject *key = PyTuple_GET_ITEM(map, j);
 		PyObject *value = PyObject_GetItem(dict, key);
-		if (value == NULL)
+                assert(PyString_Check(key));
+                /* We only care about NULLs if clear is true. */
+		if (value == NULL) {
 			PyErr_Clear();
+                        if (!clear)
+                                continue;
+                }
 		if (deref) {
-			if (value || clear) {
+                        assert(PyCell_Check(values[j]));
-				if (PyCell_GET(values[j]) != value) {
+                        if (PyCell_GET(values[j]) != value) {
-					if (PyCell_Set(values[j], value) < 0)
+                                if (PyCell_Set(values[j], value) < 0)
-						PyErr_Clear();
+                                        PyErr_Clear();
-				}
+                        }
-			}
+		} else if (values[j] != value) {
-		} else if (value != NULL || clear) {
+                        Py_XINCREF(value);
-			if (values[j] != value) {
+                        Py_XDECREF(values[j]);
-				Py_XINCREF(value);
+                        values[j] = value;
-				Py_XDECREF(values[j]);
-				values[j] = value;
-			}
 		}
 		Py_XDECREF(value);
 	}
@@ -788,8 +834,18 @@ PyFrame_FastToLocals(PyFrameObject *f)
 	if (ncells || nfreevars) {
 		map_to_dict(co->co_cellvars, ncells,
 			    locals, fast + co->co_nlocals, 1);
-		map_to_dict(co->co_freevars, nfreevars,
+                /* If the namespace is unoptimized, then one of the 
-			    locals, fast + co->co_nlocals + ncells, 1);
+                   following cases applies:
+                   1. It does not contain free variables, because it
+                      uses import * or is a top-level namespace.
+                   2. It is a class namespace.
+                   We don't want to accidentally copy free variables
+                   into the locals dict used by the class.
+                */
+                if (co->co_flags & CO_OPTIMIZED) {
+                        map_to_dict(co->co_freevars, nfreevars,
+                                    locals, fast + co->co_nlocals + ncells, 1);
+                }
 	}
 	PyErr_Restore(error_type, error_value, error_traceback);
 }