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

Victor Stinner vstinner at redhat.com
Wed Oct 31 15:57:07 EDT 2018

Ok, I proposed to rename internal header files, add an "internal_" prefix, to avoid this issue: https://github.com/python/cpython/pull/10263

My PR also adds Include/internal/ to search paths of header files.

Extract of the change:

diff --git a/Include/internal/pystate.h b/Include/internal/internal_pystate.h --- a/Include/internal/pystate.h +++ b/Include/internal/internal_pystate.h @@ -7,9 +7,9 @@ extern "C" {

#include "pystate.h" #include "pythread.h"

-#include "internal/mem.h" -#include "internal/ceval.h" -#include "internal/warnings.h" +#include "internal_pymem.h" +#include "internal_ceval.h" +#include "internal_warnings.h"

With this PR, internal_pystate.h of Include/internal/ is able to properly include pystate.h from Include/ (include the correct header file).

For practical reasons, I added Include/internal/ to all projects of the Visual Studio solution (I modified the properties common to all projects).

Again, this is where I would like to replace "internal_" with "pycore_": "internal" is too generic, there is a risk that a third party project also the same header filename. "internal_hash.h" name is quite general. There is a non-zero risk of conflict with a library. In the past, we also had issues when compiling OpenSSL for example.

Maybe we can keep "Include/internal/" directory name, but add "pycore_" rather than "internal_" to header filenames?

Victor

> > 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 PyINTERNALPYSTATEH #define PyINTERNALPYSTATEH ... #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/internalpystate.h or Include/internal/pycorepystate.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 "pycorepystate.h" instead of #include "internalpystate.h". But I have no strong preference for "pycore" or "internal", both work for me.

More information about the Python-Dev mailing list