8000 bpo-40870: Invalidate usage of some constants with ast.Name by isidentical · Pull Request #20649 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-40870: Invalidate usage of some constants with ast.Name #20649

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
Jun 6, 2020
Merged

bpo-40870: Invalidate usage of some constants with ast.Name #20649

merged 2 commits into from
Jun 6, 2020

Conversation

isidentical
Copy link
Member
@isidentical isidentical commented Jun 5, 2020 •
edited by pablogsal
Loading

pablogsal
pablogsal reviewed Jun 5, 2020
View reviewed changes
pablogsal
pablogsal reviewed Jun 5, 2020
View reviewed changes
@isidentical isidentical requested a review from pablogsal June 5, 2020 17:24
@isidentical @pablogsal
Update Misc/NEWS.d/next/Core and Builtins/2020-06-05-12-48-28.bpo-408…
6c854af 
…70.9cd2sk.rst

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
@isidentical isidentical closed this Jun 6, 2020
@isidentical isidentical reopened this Jun 6, 2020
@miss-islington
Copy link
Contributor

@isidentical: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 68874a8 into python:master Jun 6, 2020
@miss-islington
Copy link
Contributor

Thanks @isidentical for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Jun 6, 2020
@bedevere-bot
Copy link

GH-20675 is a backport of this pull request to the 3.9 branch.

@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 6, 2020
@isidentical @miss-islington
bpo-40870: Invalidate usage of some constants with ast.Name (pythonGH…
35c64ff 
…-20649)

(cherry picked from commit 68874a8)

Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>
@terryjreedy
Copy link
Member

@pablogsal the backports passed tests and are ready to merge, but need coredev approval. I don't know if the automerge label is supposed to be carried back or not, but it was not.

miss-islington added a commit that referenced this pull request Jun 6, 2020
@miss-islington @isidentical
bpo-40870: Invalidate usage of some constants with ast.Name (GH-20649)
90ee51f 
(cherry picked from commit 68874a8)

Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>
miss-islington added a commit that referenced this pull request Jun 6, 2020
@miss-islington @isidentical
bpo-40870: Invalidate usage of some constants with ast.Name (GH-20649)
83a9ba4 
(cherry picked from commit 68874a8)

Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>
@pablogsal
Copy link
Member

Thanks @terryjreedy for the ping. I have merged the backports :)

@wisp3rwind
Copy link

@isidentical , @pablogsal: Is it clear that this should be backported to 3.8? This will break some programs out in the wild unexpectedly. While I don't know the discussion leading to this PR (I suppose that ast.Name("None", ast.Load()) could be considered invalid usage in the first place), I wouldn't have expected such a change without a major version bump.

In particular, I encountered this for https://github.com/beetbox/beets/ which uses ast to implement a simple templating engine.

wisp3rwind added a commit to wisp3rwind/beets that referenced this pull request Jun 9, 2020
@wisp3rwind
compatibility with breaking changes to the ast module
16b06a5 
certainly in 3.9, currently also backported to 3.8: python/cpython#20649
@pablogsal
Copy link
Member
pablogsal commented Jun 9, 2020 •
edited
Loading

Is it clear that this should be backported to 3.8?

Yes, is a bug. ast.Name("None") is invalid in Load/Store contexts because those cannot be variable names you can load or assign to. In Python3.8 the singletons None, True and False are ast.Constant so trying to use them as names is invalid.

I wouldn't have expected such a change without a major version bump.

Although we do our best to reduce breakage, the ast module reflects the internal CPython AST and that can change between major version bumps (from 3.7 to 3.8 for example) as if this was not possible, it would prevent us to optimize the CPython VM.

wisp3rwind added a commit to wisp3rwind/beets that referenced this pull request Jun 9, 2020
@wisp3rwind
compatibility with breaking changes to the ast module
a3044be 
new in 3.10, also backported to 3.8 and 3.9: python/cpython#20649
In fact, our generation of some Literals has been invalid since Python
3.4, fix that too.
@wisp3 8000 rwind wisp3rwind mentioned this pull request Jun 9, 2020
Merged
wisp3rwind added a commit to wisp3rwind/beets that referenced this pull request Jun 9, 2020
@wisp3rwind
compatibility with breaking changes to the ast module
dab0c1f 
new in 3.10, also backported to 3.8 and 3.9: python/cpython#20649
In fact, our generation of some Literals has been invalid since Python
3.4, fix that too.
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
@isidentical @arun-mani-j
bpo-40870: Invalidate usage of some constants with ast.Name (pythonGH…
e9747c9 
…-20649)
@mweinelt mweinelt mentioned this pull request Aug 5, 2020
Merged
10 tasks
mweinelt added a commit to mweinelt/nixpkgs that referenced this pull request Aug 6, 2020
@mweinelt
beets: apply patch to fix incompatibilites with python 3.8
d55276d 
beetbox/beets#3621
python/cpython#20649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal approved these changes

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@isidentical @miss-islington @bedevere-bot @terryjreedy @pablogsal @wisp3rwind @the-knights-who-say-ni
0