On Fri, Nov 2, 2018 at 1:02 PM Victor Stinner firstname.lastname@example.org wrote:
Le ven. 2 nov. 2018 à 18:32, Neil Schemenauer email@example.com a écrit :
A simple approach would be to introduce something like Python-internal.h. If you are a Python internal unit, you can include both Python.h and Python-internal.h. We could, over time, split Python-iternal.h into smaller modular includes.
Since this discussion, I already moved most Py_BUILD_CORE in Include/internal/. I added a lot of #include "pycore_xxx.h" in C files.
I started to reach the limit with this PR which adds the pycore_object.h include to not less than 33 C files: https://github.com/python/cpython/pull/10272
I rewrote this PR to avoid the need to modify 33 C files: https://github.com/python/cpython/pull/10276/files
I added the following code to Python.h:
#ifdef Py_BUILD_CORE /* bpo-35081: Automatically include pycore_object.h in Python.h, to avoid to have to add an explicit #include "pycore_object.h" to each C file. */ # include "pycore_object.h" #endif
I'm not sure of it's a temporary workaround or not :-) Maybe a Python-internal.h would be a better solution? We can identify the most common header files needed to access "Python internals" and put them in this "common" header file.
For example, most C files using Python internals have to access _PyRuntime global variable (55 C files), so "pycore_state.h" is a good candidate.
FWIW, I'm still in favor of keeping the includes specific. For me personally it helps to narrow down dependencies, making the source easier to follow.
By the way, I don't understand the rationale to have _PyRuntime in pycore_state.h. IMHO pycore_state.h should only contain functions to get the Python thread and Python interpreter states. IMHO _PyRuntime is unrelated and should belong to a different header file, maybe pycore_runtime.h? I didn't do this change yet *because* I would have to modify the 55 files currently including it.
The runtime state isn't all that different from the thread state or the interpreter state. It's simply broader in scope, much like the interpreter state is broader in scope than the thread state. That's why I put it where I did.