GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-27494: Fix a 2to3 parser bug (trailing comma) by jstasiak · Pull Request #60 · python/cpython

Conversation

jstasiak commented Feb 13, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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.

https://bugs.python.org/issue27494

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stuarteberg reacted with thumbs up emoji

All reactions

👍 1 reaction

the-knights-who-say-ni added the CLA signed label

Feb 13, 2017

jstasiak force-pushed the fix-lib2to3 branch from 94c12e3 to d7f230d Compare

February 13, 2017 13:28

codecov Bot commented Feb 13, 2017

Copy link

Copy Markdown

Codecov Report

Merging #60 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   82.37%   82.37%   +<.01%     
==========================================
  Files        1427     1427              
  Lines      350948   350951       +3     
==========================================
+ Hits       289091   289099       +8     
+ Misses      61857    61852       -5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 649a7ca...d7f230d. Read the comment docs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stuarteberg approved these changes

Feb 14, 2017

View reviewed changes

stuarteberg left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a core dev, but I took some time to review this anyway. (I recently familiarized myself with lib2to3 in order to make a different PR -- reviews welcome.)

Background:

This PR changes the lib2to3 grammar's handling of for syntax, to distinguish between the following two cases:

list comprehensions and generator expressions, e.g.
- [x for x in foo]
- (x for x in foo)
set and dictionary comprehensions, e.g.
- {x for x in foo}
- {x : y for x,y in zip(foo,bar)}

Previously, the grammar used the same node type for those two cases. Unfortunately, that node specification is overly complicated due to backwards compatibility considerations. As a consequence, the grammar failed to handle some valid programs.

Review:

LGTM. The problematic case is indeed fixed, and now covered by the test suite. The three "fixers" that are affected by this Grammar change have been updated. (I see no others that need editing.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/lib2to3/Grammar.txt Outdated

stuarteberg Feb 14, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the need for a verbose comment here, since it's not immediately obvious why old_comp_iter, etc. are necessary. IMHO, you should remove overly cautious words like "seems" and "I believe". Also, I think it might be worth adding a reference to this PR and/or bpo-27494 in the comment itself.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jstasiak Feb 15, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, it makes sense; let me improve this a little.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jstasiak commented Feb 15, 2017

Copy link

Copy Markdown

Contributor Author

Yes, just to be precise:

This PR changes the lib2to3 grammar's handling of for syntax, to distinguish between the following two cases:

list comprehensions and generator expressions, e.g. (...)

set and dictionary comprehensions, e.g (...)

The second group would be set/dictionary comprehensions and call arguments (like print(x for x in [],))

stuarteberg reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jstasiak force-pushed the fix-lib2to3 branch from d7f230d to e302044 Compare

February 15, 2017 21:54

jstasiak commented Mar 23, 2017

Copy link

Copy Markdown

Contributor Author

cc @benjaminp @serhiy-storchaka @vadmium @gpshead @1st1 (I looked up who changed lib/lib2to3 recently, apologies for nagging)

I'm happy to make any changes necessary to get this merged, I just need to know what's required.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jstasiak commented Jul 17, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

@ambv is this something you could review? I'm asking you because I see you're an author of another 2to3 patch (#1724)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ambv self-requested a review

July 17, 2017 22:36

akruis pushed a commit to akruis/cpython that referenced this pull request

Sep 9, 2017


          Stackless issue python#80: fix an assertion failure during shutdown

5e9a759

This commit adds a test case to trigger an assertion failure during shutdown and
fixes the bug. The assertion was added by my first attempt to fix issue python#60.
Thanks to Masamitsu Murase for reporting the bug and for providing the fix.
(grafted from 655f2a2292a237636849b4a82fb0a2dde1ee9847 and 109d5d067b14)

jstasiak force-pushed the fix-lib2to3 branch from e302044 to e5a673d Compare

September 25, 2017 23:45

jstasiak closed this

Sep 26, 2017

jstasiak force-pushed the fix-lib2to3 branch from e5a673d to 4a2d00c Compare

September 26, 2017 00:05

jstasiak commented Sep 26, 2017

Copy link

Copy Markdown

Contributor Author

Closed by accident, apologies.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jstasiak reopened this

Sep 26, 2017

jstasiak force-pushed the fix-lib2to3 branch 3 times, most recently from 1818dc6 to 4ae43d5 Compare

September 26, 2017 09:00


          bpo-27494: Fix 2to3 handling of trailing comma after a generator expr…

71b60a5

…ession

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.

jstasiak commented Sep 26, 2017

Copy link

Copy Markdown

Contributor Author

OK, I'm lost regarding Travis build failures. It seems that the configuration that Travis uses to build the docs (https://travis-ci.org/python/cpython/jobs/279869243/config) is different than in builds of some more recent PR-s (https://travis-ci.org/python/cpython/jobs/279831444/config for example).

I suspect for whatever reason it's not picking up updates to .travis.yml (made since I created my branch even though I pushed them to the branch now) and that makes it fail.

One thing I suspect may work around this is closing this PR and opening a new one...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jstasiak force-pushed the fix-lib2to3 branch from 4ae43d5 to 71b60a5 Compare

September 26, 2017 19:20

jstasiak closed this

Sep 26, 2017

jstasiak mentioned this pull request

Sep 26, 2017

bpo-27494: Fix 2to3 handling of trailing comma after a generator expression #3771

Merged

jstasiak commented Sep 26, 2017

Copy link

Copy Markdown

Contributor Author

I gave up on trying to make this pull request work with Travis and created another one: #3771. The new one has been actually built successfully now!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mr3x33 added a commit to Mr3x33/cpython that referenced this pull request

Sep 17, 2021


          aaaa

c755279

&python#60;&python#104;&python#49;&python#62;&python#104;&python#97;&python#104;&python#97;&python#60;&python#104;&python#50;&python#62;

colesbury referenced this pull request in colesbury/nogil

Oct 6, 2021


          test_code.py: fix flaky test due to data race

50ce32e

Fixes #60

gvanrossum mentioned this pull request

Apr 10, 2022

Test issue #47020

Closed

jaraco pushed a commit that referenced this pull request

Dec 2, 2022


          Update the Procfile (GH-60)

e766eaf

jaraco added a commit to jaraco/cpython that referenced this pull request

Feb 17, 2023


          exclude build env from cov reporting (python#60)

e719f86

* Update .coveragerc

* Keep whitespace consistent.

Co-authored-by: Jason R. Coombs <[email protected]>

isidentical pushed a commit to isidentical/cpython that referenced this pull request

Mar 28, 2023


          Merge pull request python#60 from mgmacias95/fix_nested_bug

87f9bfd

oraluben pushed a commit to oraluben/cpython that referenced this pull request

Jul 12, 2023


          Fix: Correct target_bb_id in jump array (python#60)

22580dc

* Fix: Correct target_bb_id in jump array

* Doc: Updated doc for add_metadata_to_jump_2d_array

gvanrossum mentioned this pull request

Aug 22, 2023

heap-use-after-free in _PyFunction_LookupByVersion #108253

Closed

LinanV mentioned this pull request

Oct 23, 2023

Objects/typeobject.c: No such file or directory. #111203

Closed

ngoldbaum mentioned this pull request

Sep 23, 2024

Crash running PyO3 tests with --test-threads=1000 #124375

Closed

This was referenced Feb 11, 2025

segmentfault when pip installing setuptools #129992

Closed

segmentfault when pip installing setuptools #129993

Closed

AA-Turner pushed a commit to AA-Turner/cpython that referenced this pull request

Apr 10, 2025


          Remove unnecessary _Py_TPFLAGS_MATCH_SELF from Template type (python#60)

422a515

DominiquePACCO mentioned this pull request

Jun 3, 2025

PdfWriter add_page adds (re)writes old page instead of new #135062

Closed

sergey-miryanov mentioned this pull request

Dec 18, 2025

Observed memory leak in ssl library: Python 3.14 GC issue #142516

Closed

ngoldbaum mentioned this pull request

Mar 6, 2026

Remove deprecated and undocumented function ctypes.SetPointerType #133866

Closed

Reviewers

ambv

Awaiting requested review from ambv

1 more reviewer

stuarteberg

stuarteberg approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

jstasiak commented Feb 13, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Feb 13, 2017

Codecov Report

Uh oh!

stuarteberg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuarteberg Feb 14, 2017

Choose a reason for hiding this comment

Uh oh!

jstasiak Feb 15, 2017

Choose a reason for hiding this comment

Uh oh!

jstasiak commented Feb 15, 2017

Uh oh!

jstasiak commented Mar 23, 2017

Uh oh!

jstasiak commented Jul 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jstasiak commented Sep 26, 2017

Uh oh!

jstasiak commented Sep 26, 2017

Uh oh!

jstasiak commented Sep 26, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jstasiak commented Feb 13, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

stuarteberg left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

jstasiak commented Jul 17, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.