Skip to content

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

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

Merged
merged 4 commits into from
Oct 14, 2019

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Oct 13, 2019

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.

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!

@pablogsal
Copy link
Member Author

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

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

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

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

@ncoghlan
Copy link
Contributor

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
Copy link
Member Author

Thanks @ncoghlan for the thorough review!

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

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
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 <[email protected]>
@bedevere-bot
Copy link

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
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 <[email protected]>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants