greg/cpython
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

gh-117482: Simplify the Fix For Builtin Types Slot Wrappers (GH-122865)

Browse Source 
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. That fix has turned
out to be incomplete for some of the builtin types we haven't
been testing. I found that out while improving the tests.

A while back, @markshannon suggested a simpler fix that doesn't
have that problem, which I've already applied to 3.12 and 3.13.
I'm switching to that here. Given the potential long-term
benefits of the more complex (but still incomplete) approach,
I'll circle back to it in the future, particularly after I've improved
the tests so no corner cases slip through the cracks.

(This is effectively a "forward-port" of 716c677 from 3.13.)
This commit is contained in:
Eric Snow
2024-09-09 08:04:58 -06:00
committed by GitHub
parent b950831c94
commit d8f3c1e8f9
5 changed files with 38 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
Objects/typeobject.c
View File

@@ -314,15 +314,6 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self,
}
}
static PyTypeObject *
managed_static_type_get_def(PyTypeObject *self, int isbuiltin)
{
size_t index = managed_static_type_index_get(self);
size_t full_index = isbuiltin
? index
: index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
return &_PyRuntime.types.managed_static.types[full_index].def;
}
PyObject *
@@ -5927,6 +5918,7 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
_PyStaticType_ClearWeakRefs(interp, type);
managed_static_type_state_clear(interp, type, isbuiltin, final);
/* We leave _Py_TPFLAGS_STATIC_BUILTIN set on tp_flags. */
}
void
@@ -7939,7 +7931,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
return 0;
}
static int add_operators(PyTypeObject *, PyTypeObject *);
static int add_operators(PyTypeObject *type);
static int add_tp_new_wrapper(PyTypeObject *type);
#define COLLECTION_FLAGS (Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_MAPPING)
@@ -8104,10 +8096,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) {
@@ -8426,7 +8418,7 @@ type_ready_post_checks(PyTypeObject *type)
static int
type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
type_ready(PyTypeObject *type, int initial)
{
ASSERT_TYPE_LOCK_HELD();
@@ -8465,7 +8457,7 @@ type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
if (type_ready_set_new(type, initial) < 0) {
goto error;
}
if (type_ready_fill_dict(type, def) < 0) {
if (type_ready_fill_dict(type) < 0) {
goto error;
}
if (initial) {
@@ -8522,7 +8514,7 @@ PyType_Ready(PyTypeObject *type)
int res;
BEGIN_TYPE_LOCK();
if (!(type->tp_flags & Py_TPFLAGS_READY)) {
res = type_ready(type, NULL, 1);
res = type_ready(type, 1);
} else {
res = 0;
assert(_PyType_CheckConsistency(type));
@@ -8558,20 +8550,14 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
managed_static_type_state_init(interp, self, isbuiltin, initial);
PyTypeObject *def = managed_static_type_get_def(self, isbuiltin);
if (initial) {
memcpy(def, self, sizeof(PyTypeObject));
}
int res;
BEGIN_TYPE_LOCK();
res = type_ready(self, def, initial);
res = type_ready(self, initial);
END_TYPE_LOCK();
if (res < 0) {
_PyStaticType_ClearWeakRefs(interp, self);
managed_static_type_state_clear(interp, self, isbuiltin, initial);
}
return res;
}
@@ -11182,6 +11168,26 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
return 0;
}
static int
slot_inherited(PyTypeObject *type, pytype_slotdef *slotdef, void **slot)
{
void **slot_base = slotptr(type->tp_base, slotdef->offset);
if (slot_base == NULL || *slot != *slot_base) {
return 0;
}
/* Some slots are inherited in pairs. */
if (slot == (void *)&type->tp_hash) {
return (type->tp_richcompare == type->tp_base->tp_richcompare);
}
else if (slot == (void *)&type->tp_richcompare) {
return (type->tp_hash == type->tp_base->tp_hash);
}
/* It must be inherited (see type_ready_inherit()). */
return 1;
}
/* 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
@@ -11213,24 +11219,26 @@ 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;
/* Also ignore when the type slot has been inherited. */
if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN
&& type->tp_base != NULL
&& slot_inherited(type, p, ptr))
{
continue;
}
int r = PyDict_Contains(dict, p->name_strobj);
if (r > 0)
continue;