[Python-checkins] r70831 - in python/branches/release30-maint: Lib/symtable.py Lib/test/test_scope.py Lib/test/test_symtable.py Python/symtable.c

jeremy.hylton python-checkins at python.org
Tue Mar 31 18:17:07 CEST 2009


Author: jeremy.hylton
Date: Tue Mar 31 18:17:06 2009
New Revision: 70831

Log:
Merged revisions 70823 via svnmerge from 
svn+ssh://pythondev@svn.python.org/python/branches/py3k

................
  r70823 | jeremy.hylton | 2009-03-31 11:26:37 -0400 (Tue, 31 Mar 2009) | 29 lines
  
  Merged revisions 70801,70809 via svnmerge from 
  svn+ssh://pythondev@svn.python.org/python/trunk
  
  The merge ran into a lot of conflicts because dicts were replaced with
  sets in the Python 3 version of the symbol table.
  
  ........
    r70801 | jeremy.hylton | 2009-03-31 09:17:03 -0400 (Tue, 31 Mar 2009) | 3 lines
    
    Add is_declared_global() which distinguishes between implicit and
    explicit global variables.
  ........
    r70809 | jeremy.hylton | 2009-03-31 09:48:15 -0400 (Tue, 31 Mar 2009) | 14 lines
    
    Global statements from one function leaked into parallel functions.
    
    Re http://bugs.python.org/issue4315
    
    The symbol table used the same name dictionaries to recursively
    analyze each of its child blocks, even though the dictionaries are
    modified during analysis.  The fix is to create new temporary
    dictionaries via the analyze_child_block().  The only information that
    needs to propagate back up is the names of the free variables.
    
    Add more comments and break out a helper function.  This code doesn't
    get any easier to understand when you only look at it once a year.
  ........
................


Modified:
   python/branches/release30-maint/   (props changed)
   python/branches/release30-maint/Lib/symtable.py
   python/branches/release30-maint/Lib/test/test_scope.py
   python/branches/release30-maint/Lib/test/test_symtable.py
   python/branches/release30-maint/Python/symtable.c

Modified: python/branches/release30-maint/Lib/symtable.py
==============================================================================
--- python/branches/release30-maint/Lib/symtable.py	(original)
+++ python/branches/release30-maint/Lib/symtable.py	Tue Mar 31 18:17:06 2009
@@ -191,6 +191,9 @@
     def is_global(self):
         return bool(self.__scope in (GLOBAL_IMPLICIT, GLOBAL_EXPLICIT))
 
+    def is_declared_global(self):
+        return bool(self.__scope == GLOBAL_EXPLICIT)
+
     def is_local(self):
         return bool(self.__flags & DEF_BOUND)
 

Modified: python/branches/release30-maint/Lib/test/test_scope.py
==============================================================================
--- python/branches/release30-maint/Lib/test/test_scope.py	(original)
+++ python/branches/release30-maint/Lib/test/test_scope.py	Tue Mar 31 18:17:06 2009
@@ -618,7 +618,6 @@
         self.assertEqual(dec(), 0)
 
     def testNonLocalMethod(self):
-
         def f(x):
             class c:
                 def inc(self):
@@ -630,13 +629,36 @@
                     x -= 1
                     return x
             return c()
-
         c = f(0)
         self.assertEqual(c.inc(), 1)
         self.assertEqual(c.inc(), 2)
         self.assertEqual(c.dec(), 1)
         self.assertEqual(c.dec(), 0)
 
+    def testGlobalInParallelNestedFunctions(self):
+        # A symbol table bug leaked the global statement from one
+        # function to other nested functions in the same block.
+        # This test verifies that a global statement in the first
+        # function does not affect the second function.
+        CODE = """def f():
+    y = 1
+    def g():
+        global y
+        return y
+    def h():
+        return y + 1
+    return g, h
+y = 9
+g, h = f()
+result9 = g()
+result2 = h()
+"""
+        local_ns = {}
+        global_ns = {}
+        exec(CODE, local_ns, global_ns)
+        self.assertEqual(2, global_ns["result2"])
+        self.assertEqual(9, global_ns["result9"])
+
     def testNonLocalClass(self):
 
         def f(x):

Modified: python/branches/release30-maint/Lib/test/test_symtable.py
==============================================================================
--- python/branches/release30-maint/Lib/test/test_symtable.py	(original)
+++ python/branches/release30-maint/Lib/test/test_symtable.py	Tue Mar 31 18:17:06 2009
@@ -88,7 +88,9 @@
 
     def test_globals(self):
         self.assertTrue(self.spam.lookup("glob").is_global())
+        self.assertFalse(self.spam.lookup("glob").is_declared_global())
         self.assertTrue(self.spam.lookup("bar").is_global())
+        self.assertTrue(self.spam.lookup("bar").is_declared_global())
         self.assertFalse(self.internal.lookup("x").is_global())
         self.assertFalse(self.Mine.lookup("instance_var").is_global())
 

Modified: python/branches/release30-maint/Python/symtable.c
==============================================================================
--- python/branches/release30-maint/Python/symtable.c	(original)
+++ python/branches/release30-maint/Python/symtable.c	Tue Mar 31 18:17:06 2009
@@ -376,8 +376,9 @@
 
 /* Decide on scope of name, given flags.
 
-   The dicts passed in as arguments are modified as necessary.
-   ste is passed so that flags can be updated.
+   The namespace dictionaries may be modified to record information
+   about the new name.  For example, a new global will add an entry to
+   global.  A name that was global can be changed to local.
 */
 
 static int 
@@ -453,7 +454,7 @@
 	   explicit?  It could also be global implicit.
 	 */
 	if (global && PySet_Contains(global, name)) {
-		SET_SCOPE(scopes, name, GLOBAL_EXPLICIT);
+		SET_SCOPE(scopes, name, GLOBAL_IMPLICIT);
 		return 1;
 	}
 	if (ste->ste_nested)
@@ -627,28 +628,56 @@
 }   
 
 /* Make final symbol table decisions for block of ste.
+
    Arguments:
    ste -- current symtable entry (input/output)
-   bound -- set of variables bound in enclosing scopes (input)
+   bound -- set of variables bound in enclosing scopes (input).  bound
+       is NULL for module blocks.
    free -- set of free variables in enclosed scopes (output)
    globals -- set of declared global variables in enclosing scopes (input)
+
+   The implementation uses two mutually recursive functions,
+   analyze_block() and analyze_child_block().  analyze_block() is
+   responsible for analyzing the individual names defined in a block.
+   analyze_child_block() prepares temporary namespace dictionaries
+   used to evaluated nested blocks.
+
+   The two functions exist because a child block should see the name
+   bindings of its enclosing blocks, but those bindings should not
+   propagate back to a parent block.
 */
 
 static int
+analyze_child_block(PySTEntryObject *entry, PyObject *bound, PyObject *free, 
+		    PyObject *global, PyObject* child_free);
+
+static int
 analyze_block(PySTEntryObject *ste, PyObject *bound, PyObject *free, 
 	      PyObject *global)
 {
 	PyObject *name, *v, *local = NULL, *scopes = NULL, *newbound = NULL;
-	PyObject *newglobal = NULL, *newfree = NULL;
+	PyObject *newglobal = NULL, *newfree = NULL, *allfree = NULL;
 	int i, success = 0;
 	Py_ssize_t pos = 0;
 
-	scopes = PyDict_New();
-	if (!scopes)
-		goto error;
-	local = PySet_New(NULL);
+	local = PySet_New(NULL);  /* collect new names bound in block */
 	if (!local)
 		goto error;
+	scopes = PyDict_New();  /* collect scopes defined for each name */
+	if (!scopes)
+		goto error;
+
+	/* Allocate new global and bound variable dictionaries.  These
+	   dictionaries hold the names visible in nested blocks.  For
+	   ClassBlocks, the bound and global names are initialized
+	   before analyzing names, because class bindings aren't
+	   visible in methods.  For other blocks, they are initialized
+	   after names are analyzed.
+	 */
+
+	/* TODO(jhylton): Package these dicts in a struct so that we
+	   can write reasonable helper functions?
+	*/
 	newglobal = PySet_New(NULL);
 	if (!newglobal)
 		goto error;
@@ -665,25 +694,22 @@
 	   this one.
 	 */
 	if (ste->ste_type == ClassBlock) {
+		/* Pass down known globals */
+		if (!PyNumber_InPlaceOr(newglobal, global))
+			goto error;
+		Py_DECREF(newglobal);
 		/* Pass down previously bound symbols */
 		if (bound) {
 			if (!PyNumber_InPlaceOr(newbound, bound))
 				goto error;
 			Py_DECREF(newbound);
 		}
-		/* Pass down known globals */
-		if (!PyNumber_InPlaceOr(newglobal, global))
-			goto error;
-		Py_DECREF(newglobal);
 	}
 
-	/* Analyze symbols in current scope */
-	assert(PySTEntry_Check(ste));
-	assert(PyDict_Check(ste->ste_symbols));
 	while (PyDict_Next(ste->ste_symbols, &pos, &name, &v)) {
 		long flags = PyLong_AS_LONG(v);
-		if (!analyze_name(ste, scopes, name, flags, bound, local, free,
-				  global))
+		if (!analyze_name(ste, scopes, name, flags,
+				  bound, local, free, global))
 			goto error;
 	}
 
@@ -715,19 +741,31 @@
 			goto error;
 	}
 
-	/* Recursively call analyze_block() on each child block */
+	/* Recursively call analyze_block() on each child block.
+
+	   newbound, newglobal now contain the names visible in
+	   nested blocks.  The free variables in the children will
+	   be collected in allfree.
+	*/
+	allfree = PySet_New(NULL);
+	if (!allfree) 
+		goto error;
 	for (i = 0; i < PyList_GET_SIZE(ste->ste_children); ++i) {
 		PyObject *c = PyList_GET_ITEM(ste->ste_children, i);
 		PySTEntryObject* entry;
 		assert(c && PySTEntry_Check(c));
 		entry = (PySTEntryObject*)c;
-		if (!analyze_block(entry, newbound, newfree, newglobal))
+		if (!analyze_child_block(entry, newbound, newfree, newglobal,
+					 allfree))
 			goto error;
 		/* Check if any children have free variables */
 		if (entry->ste_free || entry->ste_child_free)
 			ste->ste_child_free = 1;
 	}
 
+	if (PyNumber_InPlaceOr(newfree, allfree) < 0)
+		goto error;
+
 	/* Check if any local variables must be converted to cell variables */
 	if (ste->ste_type == FunctionBlock && !analyze_cells(scopes, newfree,
 							     NULL))
@@ -752,12 +790,48 @@
 	Py_XDECREF(newbound);
 	Py_XDECREF(newglobal);
 	Py_XDECREF(newfree);
+	Py_XDECREF(allfree);
 	if (!success)
 		assert(PyErr_Occurred());
 	return success;
 }
 
 static int
+analyze_child_block(PySTEntryObject *entry, PyObject *bound, PyObject *free, 
+		    PyObject *global, PyObject* child_free)
+{
+	PyObject *temp_bound = NULL, *temp_global = NULL, *temp_free = NULL;
+	int success = 0;
+
+	/* Copy the bound and global dictionaries.
+
+	   These dictionary are used by all blocks enclosed by the
+	   current block.  The analyze_block() call modifies these
+	   dictionaries.
+
+	*/
+	temp_bound = PySet_New(bound);
+	if (!temp_bound)
+		goto error;
+	temp_free = PySet_New(free);
+	if (!temp_free)
+		goto error;
+	temp_global = PySet_New(global);
+	if (!temp_global)
+		goto error;
+
+	if (!analyze_block(entry, temp_bound, temp_free, temp_global))
+		goto error;
+	success = PyNumber_InPlaceOr(child_free, temp_free) >= 0;
+	success = 1;
+ error:
+	Py_XDECREF(temp_bound);
+	Py_XDECREF(temp_free);
+	Py_XDECREF(temp_global);
+	return success;
+}
+
+static int
 symtable_analyze(struct symtable *st)
 {
 	PyObject *free, *global;


More information about the Python-checkins mailing list