Skip to content

bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b. #18477

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 12, 2020
Merged

bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b. #18477

merged 2 commits into from
Feb 12, 2020

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Feb 12, 2020 •
edited by bedevere-bot
Loading

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.8 labels Feb 12, 2020
@codecov
Copy link

codecov bot commented Feb 12, 2020 •
edited
Loading

Codecov Report

Merging #18477 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #18477     +/-   ##
=========================================
  Coverage   82.11%   82.12%             
=========================================
  Files        1956     1955      -1     
  Lines      589067   583729   -5338     
  Branches    44436    44437      +1     
=========================================
- Hits       483723   479376   -4347     
+ Misses      95690    94708    -982     
+ Partials     9654     9645      -9
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 327 more

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 95905ce...f0663f0. Read the comment docs.

@pablogsal pablogsal mentioned this pull request Feb 12, 2020
Merged
6 tasks
lysnikolaou
lysnikolaou approved these changes Feb 12, 2020
View reviewed changes
Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

LGTM!

gvanrossum
gvanrossum approved these changes Feb 12, 2020
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

From a quick glance LG.

Would it make sense to add a test for e.g. ((a)).b?

@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Feb 12, 2020

We can add complex examples like ((((a+b).c)[d])(e))().f, but it would not cover anything that was not already covered by the minimal example (a).b.

@gvanrossum
Copy link
Member

gvanrossum commented Feb 12, 2020

I don't need complex example, but I'd like a single example showing ((a)).b, since IIRC there's some special code involved in removing redundant parentheses, and I feel that that special case was involved in the bug in the first place.

@gvanrossum
Copy link
Member

gvanrossum commented Feb 12, 2020

Honestly I take it back, you can land.

@serhiy-storchaka serhiy-storchaka merged commit 6e619c4 into python:master Feb 12, 2020
@miss-islington
Copy link
Contributor

miss-islington commented Feb 12, 2020

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the ast-start-pos-trailers branch February 12, 2020 20:37
@bedevere-bot
Copy link

bedevere-bot commented Feb 12, 2020

GH-18491 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Feb 12, 2020
@miss-islington @serhiy-storchaka
bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b. (…
2076d4f 
…GH-18477)

(cherry picked from commit 6e619c4)

Co-authored-by: Serhiy Storchaka <[email protected]>
@sthagen sthagen mentioned this pull request Feb 12, 2020
Merged
@lysnikolaou lysnikolaou mentioned this pull request Feb 13, 2020
Closed
colesbury referenced this pull request in colesbury/nogil Oct 6, 2021
@serhiy-storchaka @colesbury
bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b.…
90a5109 
… (GH-18477)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@lysnikolaou lysnikolaou lysnikolaou approved these changes

@isidentical isidentical isidentical approved these changes

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@serhiy-storchaka @gvanrossum @miss-islington @bedevere-bot @lysnikolaou @isidentical @the-knights-who-say-ni