Skip to content

Commit

Permalink
[3.12] gh-117482: Simplify the Fix For Builtin Types Slot Wrappers (g…
Browse files Browse the repository at this point in the history 
…h-122241)

In gh-121602, I applied a fix to a builtin types initialization bug.
That fix made sense in the context of some broader future changes,
but introduced a little bit of extra complexity.  For earlier versions
those future changes are not relevant; we can avoid the extra complexity.
Thus we can revert that earlier change and replace it with this one,
which is more focused and conceptually simpler.  This is essentially
the implementation of an idea that @markshannon pointed out to me.

Note that this change would be much smaller if we didn't have to deal
with repr compatibility for builtin types that explicitly inherit tp slots
(see expect_manually_inherited()).  The alternative is to stop
*explicitly* inheriting tp slots in static PyTypeObject values,
which is churn that we can do separately.

(cherry picked from commit 716c677, AKA gh-121932)
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently authored Jul 24, 2024
1 parent 1a104e5 commit dd4c8ac
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 35 deletions.
122 changes: 92 additions & 30 deletions Objects/typeobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,6 @@ static_builtin_index_clear(PyTypeObject *self)
}


/* In 3.13+ this is stored in _PyRuntimeState. */
static PyTypeObject static_type_defs[_Py_MAX_STATIC_BUILTIN_TYPES];

static inline PyTypeObject *
static_builtin_get_def(PyTypeObject *type)
{
size_t index = static_builtin_index_get(type);
return &static_type_defs[index];
}


static inline static_builtin_state *
static_builtin_state_get(PyInterpreterState *interp, PyTypeObject *self)
{
Expand Down Expand Up @@ -6994,7 +6983,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
return 0;
}

static int add_operators(PyTypeObject *, PyTypeObject *);
static int add_operators(PyTypeObject *);
static int add_tp_new_wrapper(PyTypeObject *type);

#define COLLECTION_FLAGS (Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_MAPPING)
Expand Down Expand Up @@ -7159,10 +7148,10 @@ type_dict_set_doc(PyTypeObject *type)


static int
type_ready_fill_dict(PyTypeObject *type, PyTypeObject *def)
type_ready_fill_dict(PyTypeObject *type)
{
/* Add type-specific descriptors to tp_dict */
if (add_operators(type, def) < 0) {
if (add_operators(type) < 0) {
return -1;
}
if (type_add_methods(type) < 0) {
Expand Down Expand Up @@ -7474,7 +7463,7 @@ type_ready_post_checks(PyTypeObject *type)


static int
type_ready(PyTypeObject *type, PyTypeObject *def, int rerunbuiltin)
type_ready(PyTypeObject *type, int rerunbuiltin)
{
_PyObject_ASSERT((PyObject *)type, !is_readying(type));
start_readying(type);
Expand Down Expand Up @@ -7511,7 +7500,7 @@ type_ready(PyTypeObject *type, PyTypeObject *def, int rerunbuiltin)
if (type_ready_set_new(type, rerunbuiltin) < 0) {
goto error;
}
if (type_ready_fill_dict(type, def) < 0) {
if (type_ready_fill_dict(type) < 0) {
goto error;
}
if (!rerunbuiltin) {
Expand Down Expand Up @@ -7563,7 +7552,7 @@ PyType_Ready(PyTypeObject *type)
type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
}

return type_ready(type, NULL, 0);
return type_ready(type, 0);
}

int
Expand Down Expand Up @@ -7593,12 +7582,7 @@ _PyStaticType_InitBuiltin(PyInterpreterState *interp, PyTypeObject *self)

static_builtin_state_init(interp, self);

PyTypeObject *def = static_builtin_get_def(self);
if (ismain) {
memcpy(def, self, sizeof(PyTypeObject));
}

int res = type_ready(self, def, !ismain);
int res = type_ready(self, !ismain);
if (res < 0) {
static_builtin_state_clear(interp, self);
}
Expand Down Expand Up @@ -10095,6 +10079,69 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
return 0;
}

static int
expect_manually_inherited(PyTypeObject *type, void **slot)
{
PyObject *typeobj = (PyObject *)type;
if (slot == (void *)&type->tp_init) {
/* This is a best-effort list of builtin exception types
that have their own tp_init function. */
if (typeobj != PyExc_BaseException
&& typeobj != PyExc_BaseExceptionGroup
&& typeobj != PyExc_ImportError
&& typeobj != PyExc_NameError
&& typeobj != PyExc_OSError
&& typeobj != PyExc_StopIteration
&& typeobj != PyExc_SyntaxError
&& typeobj != PyExc_UnicodeDecodeError
&& typeobj != PyExc_UnicodeEncodeError)
{
return 1;
}
}
else if (slot == (void *)&type->tp_str) {
/* This is a best-effort list of builtin exception types
that have their own tp_str function. */
if (typeobj == PyExc_AttributeError || typeobj == PyExc_NameError) {
return 1;
}
}
else if (slot == (void *)&type->tp_getattr
|| slot == (void *)&type->tp_getattro)
{
/* This is a best-effort list of builtin types
that have their own tp_getattr function. */
if (typeobj == PyExc_BaseException
|| type == &PyBool_Type
|| type == &PyByteArray_Type
|| type == &PyBytes_Type
|| type == &PyClassMethod_Type
|| type == &PyComplex_Type
|| type == &PyDict_Type
|| type == &PyEnum_Type
|| type == &PyFilter_Type
|| type == &PyLong_Type
|| type == &PyList_Type
|| type == &PyMap_Type
|| type == &PyMemoryView_Type
|| type == &PyProperty_Type
|| type == &PyRange_Type
|| type == &PyReversed_Type
|| type == &PySet_Type
|| type == &PySlice_Type
|| type == &PyStaticMethod_Type
|| type == &PySuper_Type
|| type == &PyTuple_Type
|| type == &PyZip_Type)
{
return 1;
}
}

/* It must be inherited (see type_ready_inherit()).. */
return 0;
}

/* This function is called by PyType_Ready() to populate the type's
dictionary with method descriptors for function slots. For each
function slot (like tp_repr) that's defined in the type, one or more
Expand Down Expand Up @@ -10126,24 +10173,39 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
infinite recursion here.) */

static int
add_operators(PyTypeObject *type, PyTypeObject *def)
add_operators(PyTypeObject *type)
{
PyObject *dict = lookup_tp_dict(type);
pytype_slotdef *p;
PyObject *descr;
void **ptr;

assert(def == NULL || (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
if (def == NULL) {
def = type;
}

for (p = slotdefs; p->name; p++) {
if (p->wrapper == NULL)
continue;
ptr = slotptr(def, p->offset);
ptr = slotptr(type, p->offset);
if (!ptr || !*ptr)
continue;
if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN
&& type->tp_base != NULL)
{
/* Also ignore when the type slot has been inherited. */
void **ptr_base = slotptr(type->tp_base, p->offset);
if (ptr_base && *ptr == *ptr_base) {
/* Ideally we would always ignore any manually inherited
slots, Which would mean inheriting the slot wrapper
using normal attribute lookup rather than keeping
a distinct copy. However, that would introduce
a slight change in behavior that could break
existing code.
In the meantime, look the other way when the definition
explicitly inherits the slot. */
if (!expect_manually_inherited(type, ptr)) {
continue;
}
}
}
int r = PyDict_Contains(dict, p->name_strobj);
if (r > 0)
continue;
Expand Down
5 changes: 0 additions & 5 deletions Tools/c-analyzer/cpython/globals-to-fix.tsv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,6 @@ Objects/sliceobject.c - _Py_EllipsisObject -
Python/instrumentation.c - _PyInstrumentation_DISABLE -
Python/instrumentation.c - _PyInstrumentation_MISSING -

##-----------------------
## other

Objects/typeobject.c - static_type_defs -


##################################
## global non-objects to fix in core code
Expand Down

0 comments on commit dd4c8ac

Please sign in to comment.