[Python-Dev] Rename Include/internals/ to Include/pycore/ (original) (raw)

Victor Stinner vstinner at redhat.com
Wed Oct 31 15🔞39 EDT 2018

Le mer. 31 oct. 2018 à 19:22, Eric Snow <ericsnowcurrently at gmail.com> 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 (...)

Ok.

> Moreover, files of this subdirectory would have > the prefix "pycore". For example, Include/objimpl.h would have a > twin: Include/pycore/pycoreobjimpl.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_.h" and just write "internal_.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/pycoreobjimpl.h if PyBUILDCORE is defined: > > #ifdef PyBUILDCORE > # include "pycore/pycoreobjimpl.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

> Second milestone: > > * Find a solution for PyLIMITEDAPI :-)

My current idea is to:

IMHO here we really want to automatically include "Include/public/.h" from "Include/.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.

Victor

More information about the Python-Dev mailing list