Skip to content

Commit 71b60a5

Browse files
committed
bpo-27494: Fix 2to3 handling of trailing comma after a generator expression
While this is a valid Python 2 and Python 3 syntax lib2to3 would choke on it: set(x for x in [],) This patch changes the grammar definition used by lib2to3 so that the actual Python syntax is supported now and backwards compatibility is preserved.
1 parent 0e950dd commit 71b60a5

File tree

7 files changed

+38
-7
lines changed

7 files changed

+38
-7
lines changed

Lib/lib2to3/Grammar.txt

+22-3
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ atom: ('(' [yield_expr|testlist_gexp] ')' |
130130
'{' [dictsetmaker] '}' |
131131
'`' testlist1 '`' |
132132
NAME | NUMBER | STRING+ | '.' '.' '.')
133-
listmaker: (test|star_expr) ( comp_for | (',' (test|star_expr))* [','] )
134-
testlist_gexp: (test|star_expr) ( comp_for | (',' (test|star_expr))* [','] )
133+
listmaker: (test|star_expr) ( old_comp_for | (',' (test|star_expr))* [','] )
134+
testlist_gexp: (test|star_expr) ( old_comp_for | (',' (test|star_expr))* [','] )
135135
lambdef: 'lambda' [varargslist] ':' test
136136
trailer: '(' [arglist] ')' | '[' subscriptlist ']' | '.' NAME
137137
subscriptlist: subscript (',' subscript)* [',']
@@ -161,9 +161,28 @@ argument: ( test [comp_for] |
161161
star_expr )
162162

163163
comp_iter: comp_for | comp_if
164-
comp_for: [ASYNC] 'for' exprlist 'in' testlist_safe [comp_iter]
164+
comp_for: [ASYNC] 'for' exprlist 'in' or_test [comp_iter]
165165
comp_if: 'if' old_test [comp_iter]
166166

167+
# See note above regarding testlist_safe to see why testlist_safe is
168+
# necessary. Unfortunately we can't use it indiscriminately in all derivations
169+
# using comp_for-like pattern because testlist_safe derivation contains
170+
# comma which clashes with trailing comma in arglist.
171+
#
172+
# The above was an issue because the parser would not follow the correct derivation
173+
# when parsing syntactically valid Python code. Since testlist_safe was created
174+
# specifically to handle list comprehensions and generator expressions enclosed
175+
# with parentheses it's safe to only use it in those. That avoids the issue
176+
# mentioned above and we can parse code like set(x for x in [],).
177+
#
178+
# Note that the syntax supported by this set of rules is not a valid Python 3
179+
# syntax, hence the prefix "old".
180+
#
181+
# See the bug report: http://bugs.python.org/issue27494
182+
old_comp_iter: old_comp_for | old_comp_if
183+
old_comp_for: [ASYNC] 'for' exprlist 'in' testlist_safe [old_comp_iter]
184+
old_comp_if: 'if' old_test [old_comp_iter]
185+
167186
testlist1: test (',' test)*
168187

169188
# not used in grammar, but may appear in "node" passed from Parser to Compiler

Lib/lib2to3/fixer_util.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ def ListComp(xp, fp, it, test=None):
101101
test.prefix = " "
102102
if_leaf = Leaf(token.NAME, "if")
103103
if_leaf.prefix = " "
104-
inner_args.append(Node(syms.comp_if, [if_leaf, test]))
105-
inner = Node(syms.listmaker, [xp, Node(syms.comp_for, inner_args)])
104+
inner_args.append(Node(syms.old_comp_if, [if_leaf, test]))
105+
inner = Node(syms.listmaker, [xp, Node(syms.old_comp_for, inner_args)])
106106
return Node(syms.atom,
107107
[Leaf(token.LBRACE, "["),
108108
inner,
@@ -208,6 +208,7 @@ def attr_chain(obj, attr):
208208
next = getattr(next, attr)
209209

210210
p0 = """for_stmt< 'for' any 'in' node=any ':' any* >
211+
| old_comp_for< 'for' any 'in' node=any any* >
211212
| comp_for< 'for' any 'in' node=any any* >
212213
"""
213214
p1 = """

Lib/lib2to3/fixes/fix_dict.py

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def transform(self, node, results):
8383
p1 = patcomp.compile_pattern(P1)
8484

8585
P2 = """for_stmt< 'for' any 'in' node=any ':' any* >
86+
| old_comp_for< 'for' any 'in' node=any any* >
8687
| comp_for< 'for' any 'in' node=any any* >
8788
"""
8889
p2 = patcomp.compile_pattern(P2)

Lib/lib2to3/fixes/fix_paren.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class FixParen(fixer_base.BaseFix):
1515
PATTERN = """
1616
atom< ('[' | '(')
1717
(listmaker< any
18-
comp_for<
18+
old_comp_for<
1919
'for' NAME 'in'
2020
target=testlist_safe< any (',' any)+ [',']
2121
>
@@ -24,7 +24,7 @@ class FixParen(fixer_base.BaseFix):
2424
>
2525
|
2626
testlist_gexp< any
27-
comp_for<
27+
old_comp_for<
2828
'for' NAME 'in'
2929
target=testlist_safe< any (',' any)+ [',']
3030
>

Lib/lib2to3/fixes/fix_xrange.py

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def transform_range(self, node, results):
5555
p1 = patcomp.compile_pattern(P1)
5656

5757
P2 = """for_stmt< 'for' any 'in' node=any ':' any* >
58+
| old_comp_for< 'for' any 'in' node=any any* >
5859
| comp_for< 'for' any 'in' node=any any* >
5960
| comparison< any 'in' node=any any*>
6061
"""

Lib/lib2to3/tests/test_parser.py

+7
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,13 @@ def test_multiline_str_literals(self):
459459
self.validate(s)
460460

461461

462+
class TestGeneratorExpressions(GrammarTest):
463+
464+
def test_trailing_comma_after_generator_expression_argument_works(self):
465+
# BPO issue 27494
466+
self.validate("set(x for x in [],)")
467+
468+
462469
def diff(fn, result):
463470
try:
464471
with open('@', 'w') as f:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
2to3 doesn't choke on a trailing comma following a generator expression
2+
anymore.

0 commit comments

Comments
 (0)