Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
[3.12] gh-104799: Move location of type_params AST fields (GH-104828)#104974
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Seems like this changes the ABI! cc@Yhg1s I don't know whether the ABI change is relevant in practice, because external users shouldn't be constructing these internal AST nodes anyway. So maybe we should decide this is a false positive. If we think this is in fact a real issue that could affect users, I would vote to revert the change on main too and leave#104799 unfixed, as it's a relatively minor issue and the AST doesn't guarantee full backwards compatibility anyway. |
AlexWaygood commentedMay 30, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Yhg1s, thoughts on this, as release manager? My preference would be to get this merged into 3.12 ASAP, as it minimizes the backwards-compatibility breakage between 3.11 and 3.12 that comes from thePEP-695 implementation. But if the failing ABI check makes that impossible, then we should revert the change that's been made on |
JelleZijlstra commentedMay 30, 2023 • edited by AlexWaygood
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by AlexWaygood
Uh oh!
There was an error while loading.Please reload this page.
@Yhg1s said on Discord that the ABI change is fine (thanks!). I'm running the script to regenerate the ABI locally now. |
Unfortunately I couldn't get the script to work. On Mac something is wrong with libmpdec
On an AWS Ubuntu instance I get |
AlexWaygood commentedMay 30, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I used the workflow Steve is demo-ing over at#105088 to regen the ABI via GitHub Actions on my CPython fork. Ithink I did it right. At least the GitHub Actions check on this PR is now passing! |
Thanks! That diff looks pretty reasonable. |
Uh oh!
There was an error while loading.Please reload this page.
(cherry picked from commitba73473)
Co-authored-by: Jelle Zijlstrajelle.zijlstra@gmail.com
Co-authored-by: Alex WaygoodAlex.Waygood@Gmail.com
__match_args__
attributes of AST nodes #104799📚 Documentation preview 📚:https://cpython-previews--104974.org.readthedocs.build/