◐ Shell
clean mode source ↗

[3.12] gh-104799: Move location of type_params AST fields (GH-104828) by miss-islington · Pull Request #104974 · python/cpython

…4828)

(cherry picked from commit ba73473)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

This was referenced

May 26, 2023

@JelleZijlstra

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

@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 the PEP-695 implementation. But if the failing ABI check makes that impossible, then we should revert the change that's been made on main (which this PR is backporting) ASAP.

@JelleZijlstra

@Yhg1s said on Discord that the ABI change is fine (thanks!). I'm running the script to regenerate the ABI locally now.

@JelleZijlstra

Unfortunately I couldn't get the script to work. On Mac something is wrong with libmpdec

gcc -I./Modules/_decimal/libmpdec -DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1 -fno-strict-overflow -DNDEBUG -g -O3 -Wall -g3 -O0 -g3 -O0  -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal  -I. -I./Include   -fPIC -fPIC -c ./Modules/_decimal/_decimal.c -o Modules/_decimal/_decimal.o
gcc -shared      Modules/_decimal/_decimal.o -lm Modules/_decimal/libmpdec/libmpdec.a  -o Modules/_decimal.cpython-312-aarch64-linux-gnu.so
/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getprec':
/src/./Modules/_decimal/_decimal.c:740: undefined reference to `mpd_getprec'
/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getemax':
/src/./Modules/_decimal/_decimal.c:741: undefined reference to `mpd_getemax'

On an AWS Ubuntu instance I get error="docker-credential-ecr-login can only be used with Amazon Elastic Container Registry.".

@AlexWaygood

I used the workflow Steve is demo-ing over at #105088 to regen the ABI via GitHub Actions on my CPython fork. I think I did it right. At least the GitHub Actions check on this PR is now passing!

@JelleZijlstra

Thanks! That diff looks pretty reasonable.

This was referenced

May 30, 2023