Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-107603: AC only includes pycore_gc.h if needed#108715
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner commented Aug 31, 2023 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Aug 31, 2023
If we convert these modules to the limited C API, see issue #85283, the Py_ARGUMENT_CLINIC_AVOID_INTERNAL_CAPI macro can be removed (from AC and from these modules). |
65a4548 to e8ea3f9Comparevstinner commented Aug 31, 2023
Oh, global objects are outdated in the main branch, see PR #108714. |
vstinner commented Aug 31, 2023
Hum. Windows fails to build Related code, I added staticstruct{PyGC_Head_this_is_not_used; PyObject_VAR_HEADPyObject*ob_item[NUM_KEYWORDS]} _kwtuple={.ob_base=PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) // error C2099: initializer is not a constant .ob_item={&_Py_ID(doc), }, // error C2099: initializer is not a constant }; |
b8efef7 to f989d08Comparef989d08 to 750b40bCompareArgument Clinic now only includes pycore_gc.h if PyGC_Head is needed, and only includes pycore_runtime.h if _Py_ID() is needed. Add Py_ARGUMENT_CLINIC_AVOID_INTERNAL_CAPI macro to opt-out for the internal C API in Argument Clinic. The following extensions avoids the internal C API by defining the macro Py_ARGUMENT_CLINIC_AVOID_INTERNAL_CAPI: * Modules/_csv.c * Modules/_multiprocessing/posixshmem.c * Modules/_testcapi/exceptions.c * Modules/grpmodule.c * Modules/syslogmodule.c
750b40b to df2c754Comparevstinner commented Aug 31, 2023
This approach doesn't work. I abandon this PR in favor of PR #108726 which is implemented differently. |
gvanrossum commented Aug 31, 2023
Victor, next time can you use a draft PR for such experiments so 17 core devs aren't bombarded with review requests and internal monologue? |
vstinner commented Aug 31, 2023
If I create a PR as draft, no notification is sent? Well, I expected this PR to be ready, but then I saw that tests failed on Windows. I wrote a new PR from scratch with a different approach to avoid this. I'm not sure why so many developers are requested for reviews, the vast majority of modified files are generated files ( |
AlexWaygood commented Aug 31, 2023
yes, code owners are only requested for review when you take it out of draft mode
I don't think the GitHub CODEOWNERS feature takes any notice of whether files are generated or not when figuring out whether people should be pinged for review -- it just looks at whether the modified files match any of the globs provided in https://github.com/python/cpython/blob/main/.github/CODEOWNERS |
Argument Clinic now only includes pycore_gc.h if PyGC_Head is needed, and only includes pycore_runtime.h if _Py_ID() is needed.
Add Py_ARGUMENT_CLINIC_AVOID_INTERNAL_CAPI macro to opt-out for the internal C API in Argument Clinic.
The following extensions avoids the internal C API by defining the macro Py_ARGUMENT_CLINIC_AVOID_INTERNAL_CAPI: