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

Jump to bottom

bpo-38469: Handle named expression scope whith global/nonlocal keywords #16755

Merged
merged 4 commits into from Oct 14, 2019
Merged

bpo-38469: Handle named expression scope whith global/nonlocal keywords #16755

merged 4 commits into from Oct 14, 2019

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Oct 13, 2019 •
edited by bedevere-bot

@pablogsal pablogsal force-pushed the bpo-38469 branch 4 times, most recently from 5db3b99 to 319b38d Compare Oct 13, 2019
@pablogsal pablogsal self-assigned this Oct 13, 2019
gvanrossum
gvanrossum reviewed Oct 13, 2019
View changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Did you force-push anything? GitHub told me to refresh the PR but there was no new commit.

Also, alas, I don't feel comfortable approving this, I don't know the symtable code well enough any more. Sorry!

Lib/test/test_named_expressions.py Outdated Show resolved Hide resolved
Python/symtable.c Outdated Show resolved Hide resolved
@pablogsal
Copy link
Member Author

pablogsal commented Oct 14, 2019

Did you force-push anything? GitHub told me to refresh the PR but there was no new commit.

Yeah, I force pushed a version just some minutes ago. Sorry for the confusion.

@pablogsal
Copy link
Member Author

pablogsal commented Oct 14, 2019

Also, alas, I don't feel comfortable approving this, I don't know the symtable code well enough any more. Sorry!

Not a problem, thanks for the review! :)

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 14, 2019

There should be existing test cases for this, so the question is "What's wrong with them"?

The testing style in the module is also to rely on exec(), rather than on the test module itself (otherwise a compile error will keep the entire test file from running)

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 14, 2019

Sorry, I retract that "exec" comment - it only applies to the tests that are explicitly checking for compilation errors

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 14, 2019

And the missing test case is also genuinely missing rather than existing in a flawed form (the global declaration test doesn't use a comprehension, the comprehension tests don't use a global)

@pablogsal @ncoghlan
Update Misc/NEWS.d/next/Core and Builtins/2019-10-13-23-41-38.bpo-384…
df21d12 
…69.9kmuQj.rst

Co-Authored-By: Nick Coghlan <ncoghlan@gmail.com>
@pablogsal
Copy link
Member Author

pablogsal commented Oct 14, 2019

Thanks @ncoghlan for the thorough review!

@pablogsal pablogsal merged commit fd5c414 into python:master Oct 14, 2019
@pablogsal pablogsal deleted the bpo-38469 branch Oct 14, 2019
@miss-islington
Copy link
Contributor

miss-islington commented Oct 14, 2019

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 14, 2019
@pablogsal @miss-islington
bpo-38469: Handle named expression scope with global/nonlocal keywords (
63983da 
pythonGH-16755)

The symbol table handing of PEP572's assignment expressions is not resolving correctly the scope of some variables in presence of global/nonlocal keywords in conjunction with comprehensions.
(cherry picked from commit fd5c414)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Oct 14, 2019

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

ambv pushed a commit that referenced this pull request Oct 14, 2019
@miss-islington @pablogsal @ambv
bpo-38469: Handle named expression scope with global/nonlocal keywords (
927f07c 
GH-16755) (#16760)

The symbol table handing of PEP572's assignment expressions is not resolving correctly the scope of some variables in presence of global/nonlocal keywords in conjunction with comprehensions.
(cherry picked from commit fd5c414)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@pablogsal
bpo-38469: Handle named expression scope with global/nonlocal keywords (
9562587 
pythonGH-16755)

The symbol table handing of PEP572's assignment expressions is not resolving correctly the scope of some variables in presence of global/nonlocal keywords in conjunction with comprehensions.
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@pablogsal @shihai1991
bpo-38469: Handle named expression scope with global/nonlocal keywords (
fe8813d 
pythonGH-16755)

The symbol table handing of PEP572's assignment expressions is not resolving correctly the scope of some variables in presence of global/nonlocal keywords in conjunction with comprehensions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvanrossum gvanrossum

@ncoghlan ncoghlan

@emilyemorehouse emilyemorehouse

Assignees

@pablogsal pablogsal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@pablogsal @ncoghlan @miss-islington @bedevere-bot @gvanrossum @the-knights-who-say-ni