better col_offsets for "for" statements with tuple unpacking #6704

benjaminp · benjaminp · commit c0ba828857fa · 2009-08-15T22:59:21.000Z
Patch from Frank Wierzbicki.
diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py
@@ -64,6 +64,10 @@ def to_tuple(t):
     "break",
     # Continue
     "continue",
+    # for statements with naked tuples (see http://bugs.python.org/issue6704)
+    "for a,b in c: pass",
+    "[(a,b) for a,b in c]",
+    "((a,b) for a,b in c)",
 ]
 
 # These are compiled through "single"
@@ -301,7 +305,7 @@ def main():
             print kind+"_results = ["
             for s in statements:
                 print repr(to_tuple(compile(s, "?", kind, 0x400)))+","
-                print "]"
+            print "]"
         print "main()"
         raise SystemExit
     test_main()
@@ -330,6 +334,9 @@ def main():
 ('Module', [('Pass', (1, 0))]),
 ('Module', [('Break', (1, 0))]),
 ('Module', [('Continue', (1, 0))]),
+('Module', [('For', (1, 0), ('Tuple', (1, 4), [('Name', (1, 4), 'a', ('Store',)), ('Name', (1, 6), 'b', ('Store',))], ('Store',)), ('Name', (1, 11), 'c', ('Load',)), [('Pass', (1, 14))], [])]),
+('Module', [('Expr', (1, 0), ('ListComp', (1, 1), ('Tuple', (1, 2), [('Name', (1, 2), 'a', ('Load',)), ('Name', (1, 4), 'b', ('Load',))], ('Load',)), [('comprehension', ('Tuple', (1, 11), [('Name', (1, 11), 'a', ('Store',)), ('Name', (1, 13), 'b', ('Store',))], ('Store',)), ('Name', (1, 18), 'c', ('Load',)), [])]))]),
+('Module', [('Expr', (1, 0), ('GeneratorExp', (1, 1), ('Tuple', (1, 2), [('Name', (1, 2), 'a', ('Load',)), ('Name', (1, 4), 'b', ('Load',))], ('Load',)), [('comprehension', ('Tuple', (1, 11), [('Name', (1, 11), 'a', ('Store',)), ('Name', (1, 13), 'b', ('Store',))], ('Store',)), ('Name', (1, 18), 'c', ('Load',)), [])]))]),
 ]
 single_results = [
 ('Interactive', [('Expr', (1, 0), ('BinOp', (1, 0), ('Num', (1, 0), 1), ('Add',), ('Num', (1, 2), 2)))]),
diff --git a/Misc/ACKS b/Misc/ACKS
@@ -794,6 +794,7 @@ Collin Winter
 Dik Winter
 Blake Winton
 Jean-Claude Wippler
+Frank Wierzbicki
 Lars Wirzenius
 Chris Withers
 Stefan Witzel
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.7 alpha 1
 Core and Builtins
 -----------------
 
+- Issue #6704: Improve the col_offset in AST for "for" statements with
+  a target of tuple unpacking.
+
 - Issue #6707: dir() on an uninitialized module caused a crash.
 
 - Issue #6540: Fixed crash for bytearray.translate() with invalid parameters.
diff --git a/Python/ast.c b/Python/ast.c
@@ -1047,7 +1047,7 @@ ast_for_listcomp(struct compiling *c, const node *n)
        list_if: 'if' test [list_iter]
        testlist_safe: test [(',' test)+ [',']]
     */
-    expr_ty elt;
+    expr_ty elt, first;
     asdl_seq *listcomps;
     int i, n_fors;
     node *ch;
@@ -1087,11 +1087,11 @@ ast_for_listcomp(struct compiling *c, const node *n)
         /* Check the # of children rather than the length of t, since
            [x for x, in ... ] has 1 element in t, but still requires a Tuple.
         */
+        first = (expr_ty)asdl_seq_GET(t, 0);
         if (NCH(for_ch) == 1)
-            lc = comprehension((expr_ty)asdl_seq_GET(t, 0), expression, NULL,
-                               c->c_arena);
+            lc = comprehension(first, expression, NULL, c->c_arena);
         else
-            lc = comprehension(Tuple(t, Store, LINENO(ch), ch->n_col_offset,
+            lc = comprehension(Tuple(t, Store, first->lineno, first->col_offset,
                                      c->c_arena),
                                expression, NULL, c->c_arena);
         if (!lc)
@@ -1226,7 +1226,7 @@ ast_for_genexp(struct compiling *c, const node *n)
     for (i = 0; i < n_fors; i++) {
         comprehension_ty ge;
         asdl_seq *t;
-        expr_ty expression;
+        expr_ty expression, first;
         node *for_ch;
         
         REQ(ch, gen_for);
@@ -1241,11 +1241,11 @@ ast_for_genexp(struct compiling *c, const node *n)
 
         /* Check the # of children rather than the length of t, since
            (x for x, in ...) has 1 element in t, but still requires a Tuple. */
+        first = (expr_ty)asdl_seq_GET(t, 0);
         if (NCH(for_ch) == 1)
-            ge = comprehension((expr_ty)asdl_seq_GET(t, 0), expression,
-                               NULL, c->c_arena);
+            ge = comprehension(first, expression, NULL, c->c_arena);
         else
-            ge = comprehension(Tuple(t, Store, LINENO(ch), ch->n_col_offset,
+            ge = comprehension(Tuple(t, Store, first->lineno, first->col_offset,
                                      c->c_arena),
                                expression, NULL, c->c_arena);
 
@@ -2844,7 +2844,7 @@ ast_for_for_stmt(struct compiling *c, const node *n)
 {
     asdl_seq *_target, *seq = NULL, *suite_seq;
     expr_ty expression;
-    expr_ty target;
+    expr_ty target, first;
     const node *node_target;
     /* for_stmt: 'for' exprlist 'in' testlist ':' suite ['else' ':' suite] */
     REQ(n, for_stmt);
@@ -2861,10 +2861,11 @@ ast_for_for_stmt(struct compiling *c, const node *n)
         return NULL;
     /* Check the # of children rather than the length of _target, since
        for x, in ... has 1 element in _target, but still requires a Tuple. */
+    first = (expr_ty)asdl_seq_GET(_target, 0);
     if (NCH(node_target) == 1)
-        target = (expr_ty)asdl_seq_GET(_target, 0);
+        target = first;
     else
-        target = Tuple(_target, Store, LINENO(n), n->n_col_offset, c->c_arena);
+        target = Tuple(_target, Store, first->lineno, first->col_offset, c->c_arena);
 
     expression = ast_for_testlist(c, CHILD(n, 3));
     if (!expression)
