Le mer. 31 oct. 2018 à 20:40, Eric Snow <ericsnowcurren...@gmail.com> a écrit : > On Mon, Oct 29, 2018 at 6:39 AM Victor Stinner <vstin...@redhat.com> wrote: > > Le lun. 29 oct. 2018 à 06:32, Benjamin Peterson <benja...@python.org> a > > écrit : > > > How does the current Include/internal/ directory fail at accomplishing > > > your goal? > > > > Hum, let me understand how I came into this issue. I tried to convert > > _PyObject_GC_TRACK() macro to a static inline function. The macro uses > > "_PyGC_generation0" which is defined earlier as: "extern PyGC_Head > > *_PyGC_generation0;". Problem: this symbol has been removed when Eric > > Snow moved it into _PyRuntime which contains "#define > > _PyGC_generation0 _PyRuntime.gc.generation0". > > (FTR, _PyGC_generation0 is in Include/internal/mem.h.)
I pushed a first commit to "cleanup and fix" pystate.h: https://github.com/python/cpython/commit/9204fb8623896cc5f68ae350784ee25e8a7b2184 * Remove _PyThreadState_Current * Replace _PyThreadState_Current with _PyRuntime.gilstate.tstate_current I prefer to be explicit about the reference to _PyRuntime. For example, in my text editor, I'm able to follow the reference to _PyRuntime. In a debugger, I'm also able to display "_PyRuntime.gilstate.tstate_current", whereas gdb fails to display "_PyThreadState_Current" value since it's a preprocessor only thing. > > Hum, how is possible that _PyObject_GC_TRACK() of objimpl.h works as > > expected? > > > > It seems like all C file using this macro explicitly uses #include > > "internal/pystate.h" which uses #include "internal/mem.h". Oh. > > > > To me, it seems wrong that a function or macro defined in > > Include/objimpl.h requires an explicit #include "internal/pystate.h". > > objimpl.h should be self-sufficient. > > I agree. :) FWIW, when I consolidated the runtime state I took the > approach of re-using existing symbols (where possible) to avoid churn. > That's why the code does not reference _PyRuntime directly. Well, we can now fix these issues. > Shouldn't _PyObject_GC_TRACK() be moved to an internal include file? I'm trying to do that, but I have the #include bug: if I have Include/objimpl.h and Include/internal/objimpl.h, the compiler is confused and picked the wrong file... And I cannot use #include "objimpl.h" to include Include/objdump.h from Include/internal/objdump.h. ... Again, that's why I would like to rename header files. I will be even worse when I will add a second directory for the stable ABI (second part of my plan). > IIRC, there were several similar cases where API (under #ifndef > Py_LIMITED_API) relies on internal runtime state and should probably > be moved out of the public headers. Any such cases should be fixed. Yep. I'm trying to fix issues one by one. I started with the simplest changes. > If any public API must rely on the internal runtime state then it > shouldn't rely on it directly like it currently does. Instead that > state should be hidden behind public API that the public headers can > access. Right? Right. Internal things must be moved to Include/internal/. > This has a similar problem to _PyObject_GC_TRACK(): it is "public" API > that relies on internal runtime state and on the source code including > the internal headers. _PyObject_GC_TRACK() is fine: since it's a private API, we can move it to Include/internal/ and require an explicit include of the internal header file. > > We cannot remove PyThreadState_GET() from the Python C API. Removing > > any function is likely going to break multiple C extensions. > > Right. There should be very few cases of public API that relies on > internal details. In this case, could we split things up? For > example: > > "Include/pystate.h" > > // Under Py_BUILD_CORE this is replaced. > #define PyThreadState_GET() PyThreadState_Get() > > "Include/internal/pystate.h" > > #undef PyThreadState_GET > #define _PyThreadState_Current _PyRuntime.gilstate.tstate_current > #define PyThreadState_GET() \ > > ((PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current)) This API is really the worse. If we do that, it's unclear if we get the optimized flavor or the PyThreadState_Get() function call... It depends if "Include/internal/pystate.h" is included or not!? My idea here is to add a new _PyThreadState_GET() macro which would only be available in the private API. It would make sure that we pick the right implementation. Maybe PyThreadState_GET() can always be an alias to PyThreadState_Get(). Just make sure that internals use _PyThreadState_GET(). Victor _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
