Move internal headers to Include/internal/

[vstinner]

BPO	35081
Nosy	@malemburg, @tim-one, @mdickinson, @scoder, @vstinner, @ericsnowcurrently, @serhiy-storchaka, @izbyshev
PRs	[WIP] bpo-35081: Include/internal/ becomes Include/pycore/ #10143 bpo-35081: Remove Py_BUILD_CORE from datetime.h #10238 bpo-35081: Move Include/pyatomic.c to Include/internal/ #10239 bpo-35081: Cleanup pystate.c and pystate.h #10240 bpo-35081: Move Py_BUILD_CORE code to internal/mem.h #10249 bpo-35081: Add pycore_ prefix to internal header files #10263 bpo-35081: Add _PyThreadState_GET() internal macro #10266 bpo-35081: Move accu.h to Include/internal/pycore_accu.h #10271 bpo-35081: Move _PyObject_GC_TRACK() to pycore_object.h #10272 bpo-35081: And pycore_lifecycle.h and pycore_pathconfig.h #10273 [WIP] bpo-35081: _testcapi is now compiled with Py_BUILD_CORE #10274 bpo-35081: Rename internal headers #10275 bpo-35081: Move _PyObject_GC_TRACK() to pycore_object.h #10276 bpo-35081: Enhance Python-ast.h and ast.h #10277 bpo-35081: Add _PyCoreConfig_AsDict() #10362 bpo-35081: Internal headers require Py_BUILD_CORE #10363 bpo-35081: Add pycore_fileutils.h #10371 bpo-35081: Remove Py_BUILD_CORE from datetime.h #10416 bpo-35081: Make some _PyGC macros internal #10507 bpo-35081: Move _PyGC_FINALIZED() back to C API #10626 bpo-35081: Add Include/internal/pycore_object.h #10640 bpo-35081: Add Include/internal/pycore_object.h #10640 bpo-35081: Add Include/internal/pycore_object.h #10640 bpo-35081: add NEWS entry for new Include/internal/pycore_*.h files #10666 bpo-35081: Add new internal headers to Makefile #10670 bpo-35081: Add _PyTuple_CAST() #10704 bpo-35081: Add Include/internal/pycore_tupleobject.h #10705 bpo-37194: Add a new public PyObject_CallNoArgs() function #13890 bpo-35081: Move dtoa.h header to the internal C API #18489 bpo-35081: Move bytes_methods.h to the internal C API #18492 bpo-35081: Move interpreteridobject.h to Include/internal/ #28969

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-30.23:33:22.200>
created_at = <Date 2018-10-27.00:42:23.909>
labels = ['interpreter-core', '3.8']
title = 'Move internal headers to Include/internal/'
updated_at = <Date 2022-01-30.23:33:22.199>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-01-30.23:33:22.199>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-30.23:33:22.200>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2018-10-27.00:42:23.909>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 35081
keywords = ['patch']
message_count = 49.0
messages = ['328619', '328620', '328623', '328854', '328920', '328921', '328922', '328923', '328942', '329006', '329009', '329026', '329029', '329030', '329031', '329035', '329037', '329062', '329082', '329086', '329362', '329398', '329509', '329608', '329693', '329739', '329812', '329814', '329819', '329830', '329955', '330045', '330067', '330068', '330163', '330164', '330187', '330218', '330304', '330326', '330328', '330334', '330399', '330400', '361917', '361920', '403979', '403997', '404057']
nosy_count = 9.0
nosy_names = ['lemburg', 'tim.peters', 'mark.dickinson', 'atuining', 'scoder', 'vstinner', 'eric.snow', 'serhiy.storchaka', 'izbyshev']
pr_nums = ['10143', '10238', '10239', '10240', '10249', '10263', '10266', '10271', '10272', '10273', '10274', '10275', '10276', '10277', '10362', '10363', '10371', '10416', '10507', '10626', '10640', '10640', '10640', '10666', '10670', '10704', '10705', '13890', '18489', '18492', '28969']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue35081'
versions = ['Python 3.8']

[vstinner]

Currently, #include "pymem.h" may include Include/pymem.h or Include/internal/pymem.h depending where is the C file (.c) and if Include/internal/ is in the header search path or not.

I propose to:

• Rename Include/internal/ to Include/pycore/
• In this subdirectory, rename xxx.h to pycore_xxx.h to avoid any risk of confusion
• Automatically include pycore_xxx.h in xxx.h if Py_BUILD_CORE is defined: this should avoid the need of explicit #include "internal/xxx.h" in C files

For example, Include/internal/pystate.h becomes Include/pycore/pycore_pystate.h.

Attached PR implements this idea.

I chose to rename "internal" subdirectory to "pycore" to prepare the addition of other subdirectories. See:

• https://pythoncapi.readthedocs.io/split_include.html
• https://pythoncapi.readthedocs.io/

Next steps:

• Move all code surrounded by #ifdef Py_BUILD_CORE from Include/.h into Include/pycore/.h: see the second commit of my PR for an example
• Move core surrounded by #ifndef Py_LIMITED_API from Include/*.h into Include/limited/ (example of filename: Include/limited/limited_object.h)

This change should be backward compatible since Include/internal/ must not be used outside CPython core. If someone does that, well, be ready for breakage :-) It's not supported.

[vstinner]

I decided to work on this issue while trying to convert the _PyObject_GC_TRACK() macro into a "static inline" function. Currently, the macro uses _PyGC_generation0 which is defined earlier by "extern PyGC_Head *_PyGC_generation0;".

Problem: _PyGC_generation0 no longer exists... Include/internal/mem.h now defines: "#define _PyGC_generation0 _PyRuntime.gc.generation0".

Include/internal/mem.h includes Include/objimpl.h, but Include/objimpl.h requires Include/internal/mem.h. The include order matters here, many header files are inter-dependent, and have two header files with the same name in Include/ and Include/internal/ causes issues depending where the #include is done...

My PR renames mem.h to pycore_objimpl.h and include pycore_objimpl.h at "the right place" in objimpl.h. Since objimpl.h controls where pycore_objimpl.h is imported, it's simpler to handle inter-dependencies simpler.

Some inter-dependencies issues are hidden by the fact the C macros don't really require functions, macros and variables in the right order. They "just work". But converting C macros to proper "static inline" functions expose many issues.

[vstinner]

See also bpo-35059 which converts C macros to static inline functions.

[vstinner]

Discussion on python-dev:
https://mail.python.org/pipermail/python-dev/2018-October/155587.html

[vstinner]

New changeset 9204fb8 by Victor Stinner in branch 'master':
bpo-35081: Cleanup pystate.c and pystate.h (GH-10240)
9204fb8

[vstinner]

New changeset 31368a4 by Victor Stinner in branch 'master':
bpo-35081: Move Include/pyatomic.c to Include/internal/ (GH-10239)
31368a4

[vstinner]

Serhiy Storchaka asked: "Would not be better to move files with the content fully surrounded by #ifdef Py_BUILD_CORE out of the Include/ directory?"

See his comment and my answer in the PR 10239:
#10239 (comment)

[vstinner]

My changes move Py_BUILD_CORE to Include/internal/. I'm not sure of the effect on the backward compatibility.

Since Python 3.7, many "Py_BUILD_CORE" functions rely on Include/internal/, like PyThreadState_GET() which uses _PyRuntime.gilstate.tstate_current.

On my Fedora 28, the python3-devel package doesn't proide Include/internal/ headers, only Include/*.h in /usr/include/python3.7m/. It seems like Include/internal/ is not usable by 3rd party modules on Fedora at least.

I understand that even if a 3rd party C extension used the Py_BUILD_CORE API, Python 3.7 already broke these extensions.

I don't want C extensions to use Py_BUILD_CORE: Py_BUILD_CORE API is really designed to only be used inside Python. If this API is used outside Python, we cannot modify the API anymore since it would break extensions. But I want to make sure that we can break this API for different reasons.

In Python 3.7, pyatomic.h is included by Python.h. In Python 3.7.0, pyatomic.h content wasn't surrounded by Py_BUILD_CORE and this header file caused multiple compilation issues: see bpo-23644 and bpo-25150. The content is now restricted to Py_BUILD_CORE since Python 3.7.1. It allows us to more easily change the implementation.

[vstinner]

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:

diff --git a/Include/internal/pystate.h b/Include/internal/pystate.h
index 38845d32ec..2ef023a9a5 100644
--- a/Include/internal/pystate.h
+++ b/Include/internal/pystate.h
@@ -1,5 +1,3 @@
-#ifndef Py_INTERNAL_PYSTATE_H
-#define Py_INTERNAL_PYSTATE_H
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -222,4 +220,3 @@ PyAPI_FUNC(void) _PyInterpreterState_DeleteExceptMain(void);
 #ifdef __cplusplus
 }
 #endif
-#endif /* !Py_INTERNAL_PYSTATE_H */

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"

[vstinner]

New changeset 2be00d9 by Victor Stinner in branch 'master':
bpo-35081: Move Py_BUILD_CORE code to internal/mem.h (GH-10249)
2be00d9

[vstinner]

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".

I proposed to rename internal header files, add an "internal_" prefix, to avoid this issue: PR 10263.

[vstinner]

New changeset 27e2d1f by Victor Stinner in branch 'master':
bpo-35081: Add pycore_ prefix to internal header files (GH-10263)
27e2d1f

[vstinner]

New changeset 50b4857 by Victor Stinner in branch 'master':
bpo-35081: Add _PyThreadState_GET() internal macro (GH-10266)
50b4857