Skip to content

Commit 716c677

Browse files
[3.13] gh-117482: Simplify the Fix For Builtin Types Slot Wrappers (gh-121932)
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.
1 parent 0952ea9 commit 716c677

File tree

2 files changed

+93
-31
lines changed

2 files changed

+93
-31
lines changed

Include/internal/pycore_typeobject.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ struct _types_runtime_state {
3333
struct {
3434
struct {
3535
PyTypeObject *type;
36-
PyTypeObject def;
3736
int64_t interp_count;
3837
} types[_Py_MAX_MANAGED_STATIC_TYPES];
3938
} managed_static;

Objects/typeobject.c

Lines changed: 93 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,6 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self,
314314
}
315315
}
316316

317-
static PyTypeObject *
318-
managed_static_type_get_def(PyTypeObject *self, int isbuiltin)
319-
{
320-
size_t index = managed_static_type_index_get(self);
321-
size_t full_index = isbuiltin
322-
? index
323-
: index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
324-
return &_PyRuntime.types.managed_static.types[full_index].def;
325-
}
326-
327317
// Also see _PyStaticType_InitBuiltin() and _PyStaticType_FiniBuiltin().
328318

329319
/* end static builtin helpers */
@@ -5678,6 +5668,7 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
56785668

56795669
_PyStaticType_ClearWeakRefs(interp, type);
56805670
managed_static_type_state_clear(interp, type, isbuiltin, final);
5671+
/* We leave _Py_TPFLAGS_STATIC_BUILTIN set on tp_flags. */
56815672
}
56825673

56835674
void
@@ -7688,7 +7679,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
76887679
return 0;
76897680
}
76907681

7691-
static int add_operators(PyTypeObject *, PyTypeObject *);
7682+
static int add_operators(PyTypeObject *);
76927683
static int add_tp_new_wrapper(PyTypeObject *type);
76937684

76947685
#define COLLECTION_FLAGS (Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_MAPPING)
@@ -7853,10 +7844,10 @@ type_dict_set_doc(PyTypeObject *type)
78537844

78547845

78557846
static int
7856-
type_ready_fill_dict(PyTypeObject *type, PyTypeObject *def)
7847+
type_ready_fill_dict(PyTypeObject *type)
78577848
{
78587849
/* Add type-specific descriptors to tp_dict */
7859-
if (add_operators(type, def) < 0) {
7850+
if (add_operators(type) < 0) {
78607851
return -1;
78617852
}
78627853
if (type_add_methods(type) < 0) {
@@ -8175,7 +8166,7 @@ type_ready_post_checks(PyTypeObject *type)
81758166

81768167

81778168
static int
8178-
type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
8169+
type_ready(PyTypeObject *type, int initial)
81798170
{
81808171
ASSERT_TYPE_LOCK_HELD();
81818172

@@ -8214,7 +8205,7 @@ type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
82148205
if (type_ready_set_new(type, initial) < 0) {
82158206
goto error;
82168207
}
8217-
if (type_ready_fill_dict(type, def) < 0) {
8208+
if (type_ready_fill_dict(type) < 0) {
82188209
goto error;
82198210
}
82208211
if (initial) {
@@ -8271,7 +8262,7 @@ PyType_Ready(PyTypeObject *type)
82718262
int res;
82728263
BEGIN_TYPE_LOCK();
82738264
if (!(type->tp_flags & Py_TPFLAGS_READY)) {
8274-
res = type_ready(type, NULL, 1);
8265+
res = type_ready(type, 1);
82758266
} else {
82768267
res = 0;
82778268
assert(_PyType_CheckConsistency(type));
@@ -8307,20 +8298,14 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
83078298

83088299
managed_static_type_state_init(interp, self, isbuiltin, initial);
83098300

8310-
PyTypeObject *def = managed_static_type_get_def(self, isbuiltin);
8311-
if (initial) {
8312-
memcpy(def, self, sizeof(PyTypeObject));
8313-
}
8314-
83158301
int res;
83168302
BEGIN_TYPE_LOCK();
8317-
res = type_ready(self, def, initial);
8303+
res = type_ready(self, initial);
83188304
END_TYPE_LOCK();
83198305
if (res < 0) {
83208306
_PyStaticType_ClearWeakRefs(interp, self);
83218307
managed_static_type_state_clear(interp, self, isbuiltin, initial);
83228308
}
8323-
83248309
return res;
83258310
}
83268311

@@ -10877,6 +10862,69 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
1087710862
return 0;
1087810863
}
1087910864

10865+
static int
10866+
expect_manually_inherited(PyTypeObject *type, void **slot)
10867+
{
10868+
PyObject *typeobj = (PyObject *)type;
10869+
if (slot == (void *)&type->tp_init) {
10870+
/* This is a best-effort list of builtin exception types
10871+
that have their own tp_init function. */
10872+
if (typeobj != PyExc_BaseException
10873+
&& typeobj != PyExc_BaseExceptionGroup
10874+
&& typeobj != PyExc_ImportError
10875+
&& typeobj != PyExc_NameError
10876+
&& typeobj != PyExc_OSError
10877+
&& typeobj != PyExc_StopIteration
10878+
&& typeobj != PyExc_SyntaxError
10879+
&& typeobj != PyExc_UnicodeDecodeError
10880+
&& typeobj != PyExc_UnicodeEncodeError)
10881+
{
10882+
return 1;
10883+
}
10884+
}
10885+
else if (slot == (void *)&type->tp_str) {
10886+
/* This is a best-effort list of builtin exception types
10887+
that have their own tp_str function. */
10888+
if (typeobj == PyExc_AttributeError || typeobj == PyExc_NameError) {
10889+
return 1;
10890+
}
10891+
}
10892+
else if (slot == (void *)&type->tp_getattr
10893+
|| slot == (void *)&type->tp_getattro)
10894+
{
10895+
/* This is a best-effort list of builtin types
10896+
that have their own tp_getattr function. */
10897+
if (typeobj == PyExc_BaseException
10898+
|| type == &PyBool_Type
10899+
|| type == &PyByteArray_Type
10900+
|| type == &PyBytes_Type
10901+
|| type == &PyClassMethod_Type
10902+
|| type == &PyComplex_Type
10903+
|| type == &PyDict_Type
10904+
|| type == &PyEnum_Type
10905+
|| type == &PyFilter_Type
10906+
|| type == &PyLong_Type
10907+
|| type == &PyList_Type
10908+
|| type == &PyMap_Type
10909+
|| type == &PyMemoryView_Type
10910+
|| type == &PyProperty_Type
10911+
|| type == &PyRange_Type
10912+
|| type == &PyReversed_Type
10913+
|| type == &PySet_Type
10914+
|| type == &PySlice_Type
10915+
|| type == &PyStaticMethod_Type
10916+
|| type == &PySuper_Type
10917+
|| type == &PyTuple_Type
10918+
|| type == &PyZip_Type)
10919+
{
10920+
return 1;
10921+
}
10922+
}
10923+
10924+
/* It must be inherited (see type_ready_inherit()).. */
10925+
return 0;
10926+
}
10927+
1088010928
/* This function is called by PyType_Ready() to populate the type's
1088110929
dictionary with method descriptors for function slots. For each
1088210930
function slot (like tp_repr) that's defined in the type, one or more
@@ -10908,24 +10956,39 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
1090810956
infinite recursion here.) */
1090910957

1091010958
static int
10911-
add_operators(PyTypeObject *type, PyTypeObject *def)
10959+
add_operators(PyTypeObject *type)
1091210960
{
1091310961
PyObject *dict = lookup_tp_dict(type);
1091410962
pytype_slotdef *p;
1091510963
PyObject *descr;
1091610964
void **ptr;
1091710965

10918-
assert(def == NULL || (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
10919-
if (def == NULL) {
10920-
def = type;
10921-
}
10922-
1092310966
for (p = slotdefs; p->name; p++) {
1092410967
if (p->wrapper == NULL)
1092510968
continue;
10926-
ptr = slotptr(def, p->offset);
10969+
ptr = slotptr(type, p->offset);
1092710970
if (!ptr || !*ptr)
1092810971
continue;
10972+
if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN
10973+
&& type->tp_base != NULL)
10974+
{
10975+
/* Also ignore when the type slot has been inherited. */
10976+
void **ptr_base = slotptr(type->tp_base, p->offset);
10977+
if (ptr_base && *ptr == *ptr_base) {
10978+
/* Ideally we would always ignore any manually inherited
10979+
slots, Which would mean inheriting the slot wrapper
10980+
using normal attribute lookup rather than keeping
10981+
a distinct copy. However, that would introduce
10982+
a slight change in behavior that could break
10983+
existing code.
10984+
10985+
In the meantime, look the other way when the definition
10986+
explicitly inherits the slot. */
10987+
if (!expect_manually_inherited(type, ptr)) {
10988+
continue;
10989+
}
10990+
}
10991+
}
1092910992
int r = PyDict_Contains(dict, p->name_strobj);
1093010993
if (r > 0)
1093110994
continue;

0 commit comments

Comments
 (0)