bpo-27494: Fix a 2to3 parser bug (trailing comma)#60
Conversation
94c12e3 to
d7f230d
Compare
February 13, 2017 13:28
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
==========================================
+ Coverage 82.37% 82.37% +<.01%
==========================================
Files 1427 1427
Lines 350948 350951 +3
==========================================
+ Hits 289091 289099 +8
+ Misses 61857 61852 -5Continue to review full report at Codecov.
|
Sorry, something went wrong.
There was a problem hiding this comment.
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.
|
Yes, just to be precise:
The second group would be set/dictionary comprehensions and call arguments (like |
Sorry, something went wrong.
d7f230d to
e302044
Compare
February 15, 2017 21:54
|
cc @benjaminp @serhiy-storchaka @vadmium @gpshead @1st1 (I looked up who changed I'm happy to make any changes necessary to get this merged, I just need to know what's required. |
Sorry, something went wrong.
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)
e302044 to
e5a673d
Compare
September 25, 2017 23:45
e5a673d to
4a2d00c
Compare
September 26, 2017 00:05
|
Closed by accident, apologies. |
Sorry, something went wrong.
1818dc6 to
4ae43d5
Compare
September 26, 2017 09:00
…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.
|
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... |
Sorry, something went wrong.
4ae43d5 to
71b60a5
Compare
September 26, 2017 19:20
|
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! |
Sorry, something went wrong.
* Update .coveragerc * Keep whitespace consistent. Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
* Fix: Correct target_bb_id in jump array * Doc: Updated doc for add_metadata_to_jump_2d_array
While this is a valid Python 2 and Python 3 syntax lib2to3 would choke
on it:
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