Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Oct 27, 2018

  • Rename Include/internal/ directory to Include/pycore/
  • Rename Include/pycore/xxx.h to Include/pycore/pycore_xxx.h
  • Include/internal/mem.h renamed to Include/pycore/pycore_objimpl.h
  • "xxx.h" now includes "pycore/pycode_xxx.h" if Py_BUILD_CORE is
    defined: it is no longer needed to include directly "pycore_xxx.h".
    For example, include "pycore/pycore_objimpl.h" from "objimpl.h".
  • objimpl.h now includes pycore_objimpl.h just after defining
    PyGC_Head type. Replace Remove _PyGC_generation0: replaced by
    _PyRuntime.gc.generation0.

https://bugs.python.org/issue35081

* Rename Include/internal/ directory to Include/pycore/
* Rename Include/pycore/xxx.h to Include/pycore/pycore_xxx.h
* Include/internal/mem.h renamed to Include/pycore/pycore_objimpl.h
* "xxx.h" now includes "pycore/pycode_xxx.h" if Py_BUILD_CORE is
  defined: it is no longer needed to include directly "pycore_xxx.h".
  For example, include "pycore/pycore_objimpl.h" from "objimpl.h".
* objimpl.h now includes pycore_objimpl.h just after defining
  PyGC_Head type. Replace Remove _PyGC_generation0: replaced by
  _PyRuntime.gc.generation0.
pystate: move Py_BUILD_CORE code into pycore_pystate.h
@vstinner
Copy link
Member Author

I'm not sure if header files in Include/pycore/ must require or not Py_BUILD_CORE to be defined.

@vstinner vstinner changed the title bpo-35081: Include/internal/ becomes Include/pycore/ [WIP] bpo-35081: Include/internal/ becomes Include/pycore/ Oct 28, 2018
@vstinner
Copy link
Member Author

I added [WIP] to my PR title and the DO-NOT-MERGE label because I wrote this PR to "show" my proposed change, but I'm not sure about the overall directory hierarchy. I would prefer to wait the approval of a few core devs before fixing/reworking this PR :-)

@ncoghlan
Copy link
Contributor

I like the direction you're taking this, and I like the idea of having an "#ifndef PY_BUILD_CORE -> compile error" stanza in each of the pycore internal headers - that way the compiler will help us out in keeping private APIs private, rather than relying on good intentions and human code review.

@vstinner
Copy link
Member Author

I like the direction you're taking this, and I like the idea of having an "#ifndef PY_BUILD_CORE -> compile error" stanza in each of the pycore internal headers - that way the compiler will help us out in keeping private APIs private, rather than relying on good intentions and human code review.

Ah thanks! I wasn't 100% sure about this change.

See discussion on python-dev. It was decided to not automatically include the internal API from the public API (Python.h), like:

#ifdef Py_BUILD_CORE
#  include "pycore/pycore_context.h"
#endif

Instead, we have to explicitly include the private header: #include "internal/pystate.h".

Since this PR implemented an approach which has been rejected, I close it.

I started to push smaller changes with the decided approach: see https://bugs.python.org/issue35081

@vstinner vstinner closed this Oct 31, 2018
@vstinner vstinner deleted the pycore branch October 31, 2018 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants