Le mer. 31 oct. 2018 à 19:22, Eric Snow firstname.lastname@example.org a écrit :
I propose a practical solution for that: Include/*.h files would only be be public API.
As we've already discussed, I'm entirely in favor of this. :) In fact, I was thinking along those same lines when I originally created "Include/internal", when working on the runtime state consolidation. When you originally shared your idea with me (2 years ago?) it made perfect sense. :)
I think we discussed that during the CPython sprint in September 2017 :-) But I only formalized a full plan at the sprint in September 2018: http://pythoncapi.readthedocs.io/
The "core" API would live in a new subdirectory: Include/pycore/*.h.
I'm mostly -0 on this. "pycore" is fine I suppose (...)
Moreover, files of this subdirectory would have the prefix "pycore_". For example, Include/objimpl.h would have a twin: Include/pycore/pycore_objimpl.h which extend the public API with "core" APIs.
I'm not sure why this is necessary. (...) https://github.com/python/cpython/blob/master/Include/internal/pystate.h#L11 #include "internal/ceval.h" Note that a few lines above in that file I include the public header file: #include "pystate.h"
The last include doesn't work: https://bugs.python.org/issue35081#msg328942
""" Include/internal/pystate.h uses #include "pystate.h" to include Include/pystate.h, but it tries to include itself (Include/internal/pystate.h) which does nothing because of "#ifndef Py_INTERNAL_PYSTATE_H #define Py_INTERNAL_PYSTATE_H ... #endif".
Remove the #ifndef #define to see the bug: (...)
Compilation fails with:
In file included from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, from ./Include/internal/pystate.h:5, ... ./Include/internal/pystate.h:5:21: error: #include nested too deeply #include "pystate.h" """
Using <pystate.h> has no effect, it stills looks for Include/internal/pystate.h.
IMHO the only solution is to use different names in Include/ and Include/internal/, at least for the header files of Include/internal/ which also exist in Include/.
Rename Include/internal/pystate.h to Include/internal/pyruntime.h, Include/internal/internal_pystate.h or Include/internal/pycore_pystate.h.
If we add Include/internal/ to the header search path (gcc -I Include/internal/), we can avoid redundant "internal/internal_<name>.h" and just write "internal_<name>.h". I proposed to rename "internal" to "pycore" to get: #include "pycore_pystate.h" instead of #include "internal_pystate.h". But I have no strong preference for "pycore" or "internal", both work for me.
I also propose to automatically load the twin: Include/objimpl.h would load Include/pycore/pycore_objimpl.h if Py_BUILD_CORE is defined:
#ifdef Py_BUILD_CORE # include "pycore/pycore_objimpl.h" #endif
During the runtime state consolidation I took a similar approach initially, though at a less granular scale, which proved to be a headache. At first I added Include/internal/Python.h and then tried to conditionally include it in Include/Python.h. That ended up confusing, problematic, and unnecessary. At Benjamin's suggestion I switched to explicitly including "internal/<...>.h" in .c files (only where necessary). The result is simpler and more clear, identifying dependencies in source files more tightly. It's also a bit more idiomatic for well-written C code.
Ok, let's try this approach. I have to add many #include, but I'm fine to be very explicit about includes. One example (internal/mem.h): https://github.com/python/cpython/pull/10249
I'm writing "try" rather than "do", because something Py_BUILD_CORE core is mixed with public headers. Like #ifdef Py_BUILD_CORE ... #else ... #endif. See datetime.h for an example. It's not easy to separate both implementations. https://github.com/python/cpython/pull/10238
- Find a solution for Py_LIMITED_API :-)
My current idea is to:
* Remove all "#ifndef Py_LIMITED_API" and "#ifdef Py_BUILD_CORE" from Include/*.h * Move "#ifndef Py_LIMITED_API" code to Include/public/ * Include "Include/public/<name>.h" from "Include/<name>.h"
IMHO here we really want to automatically include "Include/public/<name>.h" from "Include/<name>.h" to not impact the public API in any way.
Py_BUILD_CORE is very different: it's only consumed inside Python, so it's fine to "break the API" and force to add new includes.