[Python-Dev] Rename Include/internals/ to Include/pycore/

[Victor Stinner]

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/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


> > Second milestone:
> >
> > * 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.

Victor