[3.12] gh-117482: Simplify the Fix For Builtin Types Slot Wrappers (gh-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 716c6771fcfd3be90bba9f888a579b36c02cdb13, AKA gh-121932)

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 1436b225f375a041b1d106c5749fa03830be2c78..2489d502ae7899c955eaa54c6f558dd914a28881 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -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)
 {
@@ -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)
@@ -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) {
@@ -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);
@@ -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) {
@@ -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
@@ -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);
     }
@@ -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
@@ -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;

diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv
index 62b5d882965b131b98384f192fb9f10c46080c87..ac60ee4e1416a7ccc46a53c7ec67b70a81a35a27 100644 (file)
--- a/Tools/c-analyzer/cpython/globals-to-fix.tsv
+++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv
@@ -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