Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2to3 parser failure caused by a comma after a generator expression #71681

Closed
jstasiak mannequin opened this issue Jul 12, 2016 · 13 comments
Closed

2to3 parser failure caused by a comma after a generator expression #71681

jstasiak mannequin opened this issue Jul 12, 2016 · 13 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-2to3 type-bug An unexpected behavior, bug, or error

Comments

@jstasiak
Copy link
Mannequin

jstasiak mannequin commented Jul 12, 2016

BPO 27494
Nosy @benjaminp, @ambv, @serhiy-storchaka, @jstasiak, @jayvdb, @csabella, @miss-islington
PRs
  • bpo-27494: Fix a 2to3 parser bug (trailing comma) #60
  • bpo-27494: Fix 2to3 handling of trailing comma after a generator expression #3771
  • Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (#3771)" #8241
  • [3.7] Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (GH-8241) #8580
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-07-31.06:34:34.045>
    created_at = <Date 2016-07-12.10:29:13.785>
    labels = ['3.7', '3.8', 'type-bug', 'expert-2to3']
    title = '2to3 parser failure caused by a comma after a generator expression'
    updated_at = <Date 2018-08-09.13:19:59.882>
    user = 'https://github.com/jstasiak'

    bugs.python.org fields:

    activity = <Date 2018-08-09.13:19:59.882>
    actor = 'lukasz.langa'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-07-31.06:34:34.045>
    closer = 'serhiy.storchaka'
    components = ['2to3 (2.x to 3.x conversion tool)']
    creation = <Date 2016-07-12.10:29:13.785>
    creator = 'jstasiak'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 27494
    keywords = ['patch']
    message_count = 13.0
    messages = ['270236', '295246', '303058', '303500', '303744', '306146', '306269', '322731', '322734', '322760', '322768', '322780', '323321']
    nosy_count = 7.0
    nosy_names = ['benjamin.peterson', 'lukasz.langa', 'serhiy.storchaka', 'jstasiak', 'jayvdb', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['60', '3771', '8241', '8580']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue27494'
    versions = ['Python 3.7', 'Python 3.8']

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Jul 12, 2016

    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: smarkets/flake8-strict#9 (project using lib2to3 parser)

    @jstasiak jstasiak mannequin added type-crash A hard crash of the interpreter, possibly with a core dump topic-2to3 labels Jul 12, 2016
    @jstasiak jstasiak mannequin added the 3.7 (EOL) end of life label Feb 15, 2017
    @csabella
    Copy link
    Contributor

    csabella commented Jun 6, 2017

    Adding benjamin.peterson as he is listed as the expert for 2to3.

    @serhiy-storchaka
    Copy link
    Member

    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.

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Oct 1, 2017

    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.

    @benjaminp
    Copy link
    Contributor

    New changeset af810b3 by Benjamin Peterson (Jakub Stasiak) in branch 'master':
    closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (bpo-3771)
    af810b3

    @serhiy-storchaka
    Copy link
    Member

    Actually this syntax isn't allowed by the Python language specification. See bpo-32012 for fixing the Python parser.

    @serhiy-storchaka
    Copy link
    Member

    Since this syntax never was valid according to the Python language specification, and it causes SyntaxError in 3.7 (see bpo-32012), I think that this change should be reverted.

    @serhiy-storchaka serhiy-storchaka added 3.8 only security fixes type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 11, 2018
    @serhiy-storchaka
    Copy link
    Member

    New changeset 4b8a7f5 by Serhiy Storchaka in branch 'master':
    Revert "closes bpo-27494: Fix 2to3 handling of trailing comma after a generator expression (GH-3771)" (bpo-8241)
    4b8a7f5

    @miss-islington
    Copy link
    Contributor

    New changeset 9ecbe33 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)
    9ecbe33

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Jul 31, 2018

    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 #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.

    @serhiy-storchaka
    Copy link
    Member

    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

    @jstasiak
    Copy link
    Mannequin Author

    jstasiak mannequin commented Jul 31, 2018

    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.

    @ambv
    Copy link
    Contributor

    ambv commented Aug 9, 2018

    This is one of those issues where it's clear that the parser and 2to3 should be separate.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-2to3 type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants