Fix for SF bug [ #471928 ] global made w/nested list comprehensions

The symbol table pass didn't have an explicit case for the list_iter
node which is used only for a nested list comprehension.  As a result,
the target of the list comprehension was treated as a use instead of
an assignment.  Fix is to add a case to symtable_node() to handle
list_iter.

Also, rework and document a couple of the subtler implementation
issues in the symbol table pass.  The symtable_node() switch statement
depends on falling through the last several cases, in order to handle
some of the more complicated nodes like atom.  Add a comment
explaining the behavior before the first fall through case.  Add a
comment /* fall through */ at the end of case so that it is explicitly
marked as such.

Move the for_stmt case out of the fall through logic, which simplifies
both for_stmt and default.  (The default used the local variable start
to skip the first three nodes of a for_stmt when it fell through.)

Rename the flag argument to symtable_assign() to def_flag and add a
comment explaining its use:

   The third argument to symatble_assign() is a flag to be passed to
   symtable_add_def() if it is eventually called.  The flag is useful
   to specify the particular type of assignment that should be
   recorded, e.g. an assignment caused by import.

diff --git a/Python/compile.c b/Python/compile.c
index 537aaacb530b26604666c66081480107db04332d..f2cd59effb4e14896da1ed485acac74ce5b4d8fd 100644 (file)
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -4991,7 +4991,7 @@ look_for_yield(node *n)
 static void
 symtable_node(struct symtable *st, node *n)
 {
-       int i, start = 0;
+       int i;
 
  loop:
        switch (TYPE(n)) {
@@ -5105,36 +5105,62 @@ symtable_node(struct symtable *st, node *n)
                        }
                }
                goto loop;
-               /* watchout for fall-through logic below */
+       case list_iter:
+               n = CHILD(n, 0);
+               if (TYPE(n) == list_for) {
+                       st->st_tmpname++;
+                       symtable_list_comprehension(st, n);
+                       st->st_tmpname--;
+               } else {
+                       REQ(n, list_if);
+                       symtable_node(st, CHILD(n, 1));
+                       if (NCH(n) == 3) {
+                               n = CHILD(n, 2); 
+                               goto loop;
+                       }
+               }
+               break;
+       case for_stmt:
+               symtable_assign(st, CHILD(n, 1), 0);
+               for (i = 3; i < NCH(n); ++i)
+                       if (TYPE(CHILD(n, i)) >= single_input)
+                               symtable_node(st, CHILD(n, i));
+               break;
+       /* The remaining cases fall through to default except in
+          special circumstances.  This requires the individual cases
+          to be coded with great care, even though they look like
+          rather innocuous.  Each case must double-check TYPE(n).
+       */
        case argument:
-               if (NCH(n) == 3) {
+               if (TYPE(n) == argument && NCH(n) == 3) {
                        n = CHILD(n, 2);
                        goto loop;
                }
+               /* fall through */
        case listmaker:
                if (NCH(n) > 1 && TYPE(CHILD(n, 1)) == list_for) {
                        st->st_tmpname++;
                        symtable_list_comprehension(st, CHILD(n, 1));
                        symtable_node(st, CHILD(n, 0));
                        st->st_tmpname--;
-                       return;
+                       break;
                }
+               /* fall through */
        case atom:
                if (TYPE(n) == atom && TYPE(CHILD(n, 0)) == NAME) {
                        symtable_add_use(st, STR(CHILD(n, 0)));
                        break;
                }
-       case for_stmt:
-               if (TYPE(n) == for_stmt) {
-                       symtable_assign(st, CHILD(n, 1), 0);
-                       start = 3;
-               }
+               /* fall through */
        default:
+               /* Walk over every non-token child with a special case
+                  for one child.
+               */
                if (NCH(n) == 1) {
                        n = CHILD(n, 0);
                        goto loop;
                }
-               for (i = start; i < NCH(n); ++i)
+               for (i = 0; i < NCH(n); ++i)
                        if (TYPE(CHILD(n, i)) >= single_input)
                                symtable_node(st, CHILD(n, i));
        }
@@ -5370,8 +5396,14 @@ symtable_import(struct symtable *st, node *n)
        }
 }
 
+/* The third argument to symatble_assign() is a flag to be passed to
+   symtable_add_def() if it is eventually called.  The flag is useful
+   to specify the particular type of assignment that should be
+   recorded, e.g. an assignment caused by import.
+ */
+
 static void 
-symtable_assign(struct symtable *st, node *n, int flag)
+symtable_assign(struct symtable *st, node *n, int def_flag)
 {
        node *tmp;
        int i;
@@ -5403,7 +5435,7 @@ symtable_assign(struct symtable *st, node *n, int flag)
                        return;
                } else {
                        for (i = 0; i < NCH(n); i += 2)
-                               symtable_assign(st, CHILD(n, i), flag);
+                               symtable_assign(st, CHILD(n, i), def_flag);
                }
                return;
        case exprlist:
@@ -5415,7 +5447,7 @@ symtable_assign(struct symtable *st, node *n, int flag)
                else {
                        int i;
                        for (i = 0; i < NCH(n); i += 2)
-                               symtable_assign(st, CHILD(n, i), flag);
+                               symtable_assign(st, CHILD(n, i), def_flag);
                        return;
                }
        case atom:
@@ -5426,24 +5458,24 @@ symtable_assign(struct symtable *st, node *n, int flag)
                } else if (TYPE(tmp) == NAME) {
                        if (strcmp(STR(tmp), "__debug__") == 0)
                                symtable_warn(st, ASSIGN_DEBUG);
-                       symtable_add_def(st, STR(tmp), DEF_LOCAL | flag);
+                       symtable_add_def(st, STR(tmp), DEF_LOCAL | def_flag);
                }
                return;
        case dotted_as_name:
                if (NCH(n) == 3)
                        symtable_add_def(st, STR(CHILD(n, 2)),
-                                        DEF_LOCAL | flag);
+                                        DEF_LOCAL | def_flag);
                else
                        symtable_add_def(st,
                                         STR(CHILD(CHILD(n,
                                                         0), 0)),
-                                        DEF_LOCAL | flag);
+                                        DEF_LOCAL | def_flag);
                return;
        case dotted_name:
-               symtable_add_def(st, STR(CHILD(n, 0)), DEF_LOCAL | flag);
+               symtable_add_def(st, STR(CHILD(n, 0)), DEF_LOCAL | def_flag);
                return;
        case NAME:
-               symtable_add_def(st, STR(n), DEF_LOCAL | flag);
+               symtable_add_def(st, STR(n), DEF_LOCAL | def_flag);
                return;
        default:
                if (NCH(n) == 0)
@@ -5456,6 +5488,6 @@ symtable_assign(struct symtable *st, node *n, int flag)
                   which will be caught in the next pass. */
                for (i = 0; i < NCH(n); ++i)
                        if (TYPE(CHILD(n, i)) >= single_input)
-                               symtable_assign(st, CHILD(n, i), flag);
+                               symtable_assign(st, CHILD(n, i), def_flag);
        }
 }