GeezerVison is showing github.com — gh-107603: AC only includes pycore_gc.h if needed by vstinner · Pull Request #108715 · python/cpython
Conversation
vstinner
commented
Aug 31, 2023
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
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:
- Modules/_csv.c
- Modules/_multiprocessing/posixshmem.c
- Modules/_testcapi/exceptions.c
- Modules/grpmodule.c
- Modules/syslogmodule.c
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
vstinner
requested review from
a team,
1st1,
AlexWaygood,
Fidget-Spinner,
JelleZijlstra,
abalkin,
asvetlov,
berkerpeksag,
brettcannon,
corona10,
ericsnowcurrently,
erlend-aasland,
ethanfurman,
gvanrossum,
kumaraditya303,
ncoghlan,
pganssle,
rhettinger,
tiran,
warsaw and
willingc
as code owners
August 31, 2023 11:52
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
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).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Oh, global objects are outdated in the main branch, see PR #108714.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Hum. Windows fails to build Modules\_testinternalcapi.c:
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(30,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(31,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(85,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(86,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(152,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(153,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(211,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Error: D:\a\cpython\cpython\Modules\clinic\_testinternalcapi.c.h(212,20): error C2099: initializer is not a constant [D:\a\cpython\cpython\PCbuild\_testinternalcapi.vcxproj]
Related code, I added error C2099: initializer is not a constant comments:
static struct {
PyGC_Head _this_is_not_used;
PyObject_VAR_HEAD
PyObject *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
};
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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:
* Modules/_csv.c
* Modules/_multiprocessing/posixshmem.c
* Modules/_testcapi/exceptions.c
* Modules/grpmodule.c
* Modules/syslogmodule.c
|
This approach doesn't work. I abandon this PR in favor of PR #108726 which is implemented differently.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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?
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 (*/clinic/*.h files generated by Argument Clinic).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
If I create a PR as draft, no notification is sent?
yes, code owners are only requested for review when you take it out of draft mode
I'm not sure why so many developers are requested for reviews, the vast majority of modified files are generated files (*/clinic/*.h files generated by Argument Clinic).
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
|
vstinner reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|