classification
Title: 2to3 parser failure caused by a comma after a generator expression
Type: behavior Stage: resolved
Components: 2to3 (2.x to 3.x conversion tool) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, cheryl.sabella, jayvdb, jstasiak, lukasz.langa, miss-islington, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-07-12 10:29 by jstasiak, last changed 2018-08-09 13:19 by lukasz.langa. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 60 closed jstasiak, 2017-02-15 21:42
PR 3771 merged jstasiak, 2017-09-26 19:24
PR 8241 merged serhiy.storchaka, 2018-07-11 07:06
PR 8580 merged miss-islington, 2018-07-31 06:34
Messages (13)
msg270236 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2016-07-12 10:29
Test file (test.py):

print(set(x for x in range(2),))

Python runs it nicely:

% python2 test.py 
set([0, 1])
% python3 test.py
{0, 1}

2to3 parser (on both Python 2.7.11 and 3.5.2) chokes on it though:

% /usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/bin/2to3 test.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Can't parse test.py: ParseError: bad input: type=8, value=u')', context=('', (1, 30))
RefactoringTool: No files need to be modified.
RefactoringTool: There was 1 error:
RefactoringTool: Can't parse test.py: ParseError: bad input: type=8, value=u')', context=('', (1, 30))

% /usr/local/Cellar/python3/3.5.2/Frameworks/Python.framework/Versions/3.5/bin/2to3 test.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Can't parse test.py: ParseError: bad input: type=8, value=')', context=('', (1, 30))
RefactoringTool: No files need to be modified.
RefactoringTool: There was 1 error:
RefactoringTool: Can't parse test.py: ParseError: bad input: type=8, value=')', context=('', (1, 30))


For reference: https://github.com/smarkets/flake8-strict/issues/9 (project using lib2to3 parser)
msg295246 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python triager) Date: 2017-06-06 11:10
Adding benjamin.peterson as he is listed as the expert for 2to3.
msg303058 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-26 18:11
set(x for x in range(2),) can be interpreted as set(x for x in (range(2),)).

Wouldn't be better to forbid such ambiguous syntax? The trailing comma in argument list is supported because it helps to add new arguments (or temporary comment out arguments).

    foo(x,
        y,
        #z,
       )

But set(x for x in range(2),) is not syntactically valid if add an argument after the comma. Parenthesis around a generator expression can be omitted only if it is the only argument in a function call. I think that it would be better to forbid a trailing comma in this case.
msg303500 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2017-10-01 22:01
By "forbid" do you mean "forbid in Python" (as in change Python syntax)? I like the idea but that seems like a more serious change and 2to3 arguably needs to handle code targeting older Python versions anyway.
msg303744 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-10-05 07:10
New changeset af810b35b494ef1d255d4bf340b92a9dad446995 by Benjamin Peterson (Jakub Stasiak) in branch 'master':
closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (#3771)
https://github.com/python/cpython/commit/af810b35b494ef1d255d4bf340b92a9dad446995
msg306146 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-13 07:39
Actually this syntax isn't allowed by the Python language specification. See issue32012 for fixing the Python parser.
msg306269 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-15 14:17
Since this syntax never was valid according to the Python language specification, and it causes SyntaxError in 3.7 (see issue32012), I think that this change should be reverted.
msg322731 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-31 06:34
New changeset 4b8a7f51da224d1a0ad8159935f78ba4e6e16037 by Serhiy Storchaka in branch 'master':
 Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (#8241)
https://github.com/python/cpython/commit/4b8a7f51da224d1a0ad8159935f78ba4e6e16037
msg322734 - (view) Author: miss-islington (miss-islington) Date: 2018-07-31 06:52
New changeset 9ecbe3321f7bb3726017a053e583ca507d4453fc by Miss Islington (bot) in branch '3.7':
Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (GH-8241)
https://github.com/python/cpython/commit/9ecbe3321f7bb3726017a053e583ca507d4453fc
msg322760 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2018-07-31 12:18
Apologies for only responding now, I've not received any notifications after my original pull request had been merged. I only learned about the change being reverted from https://github.com/python/cpython/pull/8580, so let me leave my two cents here:

I don't think the syntax not being valid (formally - since forever, practically - since 3.7) is good enough reason to make (lib)2to3 reject it. 2to3 is supposed to handle old syntax, isn't it? I'd argue that since it is (or was) possible to use this syntax in Python 2.x it should be handled gracefully.
msg322768 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-31 13:00
Sorry, I missed that you didn't receive a notification about creating of the reverting PR. I should announce this explicitly.

2to3 handles the old syntax, but a comma after a generator expression was not a valid old syntax. With your patch it accepted this syntax and produced invalid Python program.

$ ./python -m lib2to3 -w t9.py
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Refactored t9.py
--- t9.py       (original)
+++ t9.py       (refactored)
@@ -1 +1 @@
-print(set(x for x in range(2),))
+print((set(x for x in list(range(2)),)))
RefactoringTool: Files that were modified:
RefactoringTool: t9.py
$ ./python t9.py 
  File "t9.py", line 1
    print((set(x for x in list(range(2)),)))
              ^
SyntaxError: Generator expression must be parenthesized
msg322780 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2018-07-31 14:16
I appreciate the example, but I'd claim that's a "missing fixer" issue, not a "parser accepts too much" issue. Considering the syntax wasn't ambiguous (I think) and had been accepted before 3.7 I'll remain not totally convinced here.
msg323321 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2018-08-09 13:19
This is one of those issues where it's clear that the parser and 2to3 should be separate.
History
Date User Action Args
2018-08-09 13:19:59lukasz.langasetnosy: + lukasz.langa
messages: + msg323321
2018-07-31 14:16:45jstasiaksetmessages: + msg322780
2018-07-31 13:00:26serhiy.storchakasetmessages: + msg322768
2018-07-31 12:18:40jstasiaksetmessages: + msg322760
2018-07-31 06:52:51miss-islingtonsetnosy: + miss-islington
messages: + msg322734
2018-07-31 06:34:46miss-islingtonsetpull_requests: + pull_request8088
2018-07-31 06:34:34serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg322731

stage: patch review -> resolved
2018-07-11 07:07:27serhiy.storchakasettype: crash -> behavior
versions: + Python 3.8, - Python 2.7, Python 3.5, Python 3.6
2018-07-11 07:06:32serhiy.storchakasetstage: patch review
pull_requests: + pull_request7773
2017-11-15 14:17:32serhiy.storchakasetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg306269

stage: resolved -> (no value)
2017-11-13 07:39:22serhiy.storchakasetmessages: + msg306146
2017-10-05 07:10:11benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg303744

stage: patch review -> resolved
2017-10-01 22:01:07jstasiaksetmessages: + msg303500
2017-09-26 19:24:12jstasiaksetkeywords: + patch
stage: patch review
pull_requests: + pull_request3758
2017-09-26 18:11:47serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg303058
2017-06-06 11:10:55cheryl.sabellasetnosy: + cheryl.sabella, benjamin.peterson
messages: + msg295246
2017-02-15 22:03:42jstasiaksetversions: + Python 3.6, Python 3.7
2017-02-15 21:42:37jstasiaksetpull_requests: + pull_request87
2016-07-12 10:34:44jayvdbsetnosy: + jayvdb
2016-07-12 10:29:13jstasiakcreate