gh-94673: Clarify About Runtime State Related to Static Builtin Types#117761
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ericsnowcurrently changed the title 
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
Hm. With suitable explanation we may not need the runtime counter -- we can just explain that the index is assigned by the main thread based upon its counter.
Include/internal/pycore_typeobject.h Outdated
| Those objects are stored in the PyInterpreterState.types.builtins | ||
| array, at the index corresponding to each specific static builtin | ||
| type. That index (a size_t value) is stored in the tp_subclasses | ||
| field (defined as PyObject*). We re-purposed the now-unused |
Objects/typeobject.c Outdated
| { | ||
| if (!static_builtin_index_is_set(self)){ | ||
| static_builtin_index_set(self, interp->types.num_builtins_initialized); | ||
| assert(_Py_IsMainInterpreter(interp)); |
There was a problem hiding this comment.
In static_builtin_state_clear() there's a check for this rather than an assert. Maybe it should be an if here too, for symmetry? (Maybe the assert should be !static_builtin_index_is_set()?)
There was a problem hiding this comment.
Good point. I'll flip it around.
Objects/typeobject.c Outdated
| if (!static_builtin_index_is_set(self)){ | ||
| static_builtin_index_set(self, interp->types.num_builtins_initialized); | ||
| assert(_Py_IsMainInterpreter(interp)); | ||
| assert(interp->types.num_builtins_initialized== \ |
There was a problem hiding this comment.
It's C. :-)
| assert(interp->types.num_builtins_initialized== \ | |
| assert(interp->types.num_builtins_initialized== |
Objects/typeobject.c Outdated
| } | ||
| else{ | ||
| assert(!_Py_IsMainInterpreter(interp)); | ||
| assert(static_builtin_index_get(self) == \ |
There was a problem hiding this comment.
| assert(static_builtin_index_get(self) == \ | |
| assert(static_builtin_index_get(self) == |
This reverts commit b8d622e.
ericsnowcurrently commented Apr 12, 2024
Good point. I've dropped the extra counter. |
… Types (pythongh-117761) Guido pointed out to me that some details about the per-interpreter state for the builtin types aren't especially clear. I'm addressing that by: * adding a comment explaining that state * adding some asserts to point out the relationship between each index and the interp/global runtime state
erlend-aasland commented Apr 17, 2024
After this PR, |
3 participants
Guido pointed out to me that some details about the per-interpreter state for the builtin types aren't especially clear. I'm addressing that by:
_PyRuntimeState.types.next_builtin_indexRegarding
next_builtin_index, it replaces our use of the main interpreter'sinterp->types.num_builtins_initializedas the next index. Even though it's technically redundant, the distinct global runtime field helps clarify explicitly that each index is from global state.