https://github.com/python/cpython/commit/d8f3c1e8f9c384fa6a473c3923f3b26ad6257cac
commit: d8f3c1e8f9c384fa6a473c3923f3b26ad6257cac
branch: main
author: Eric Snow <[email protected]>
committer: encukou <[email protected]>
date: 2024-09-09T16:04:58+02:00
summary:

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

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.)

files:
M Include/internal/pycore_typeobject.h
M Lib/test/test_doctest/test_doctest.py
M Lib/test/test_embed.py
M Lib/test/test_types.py
M Objects/typeobject.c

diff --git a/Include/internal/pycore_typeobject.h 
b/Include/internal/pycore_typeobject.h
index 8ba635c5016380..ca5a1e2adb4787 100644
--- a/Include/internal/pycore_typeobject.h
+++ b/Include/internal/pycore_typeobject.h
@@ -33,7 +33,6 @@ struct _types_runtime_state {
     struct {
         struct {
             PyTypeObject *type;
-            PyTypeObject def;
             int64_t interp_count;
         } types[_Py_MAX_MANAGED_STATIC_TYPES];
     } managed_static;
diff --git a/Lib/test/test_doctest/test_doctest.py 
b/Lib/test/test_doctest/test_doctest.py
index 3e93a3bba283c2..171412cb7cb08c 100644
--- a/Lib/test/test_doctest/test_doctest.py
+++ b/Lib/test/test_doctest/test_doctest.py
@@ -738,7 +738,7 @@ def non_Python_modules(): r"""
 
     >>> import builtins
     >>> tests = doctest.DocTestFinder().find(builtins)
-    >>> 830 < len(tests) < 860 # approximate number of objects with docstrings
+    >>> 750 < len(tests) < 800 # approximate number of objects with docstrings
     True
     >>> real_tests = [t for t in tests if len(t.examples) > 0]
     >>> len(real_tests) # objects that actually have doctests
diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
index 4962586379a223..6790326a2afa47 100644
--- a/Lib/test/test_embed.py
+++ b/Lib/test/test_embed.py
@@ -416,7 +416,6 @@ def test_datetime_reset_strptime(self):
         out, err = self.run_embedded_interpreter("test_repeated_init_exec", 
code)
         self.assertEqual(out, '20000101\n' * INIT_LOOPS)
 
-    @unittest.skip('inheritance across re-init is currently broken; see 
gh-117482')
     def test_static_types_inherited_slots(self):
         script = textwrap.dedent("""
             import test.support
diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index 2ee46601f4adc6..3c9e33e3c9dbfc 100644
--- a/Lib/test/test_types.py
+++ b/Lib/test/test_types.py
@@ -2410,9 +2410,6 @@ def test_static_types_inherited_slots(self):
         def collate_results(raw):
             results = {}
             for cls, attr, wrapper in raw:
-                # XXX This should not be necessary.
-                if cls == repr(bool) and attr in self.NUMERIC_METHODS:
-                    continue
                 key = cls, attr
                 assert key not in results, (results, key, wrapper)
                 results[key] = wrapper
@@ -2433,14 +2430,7 @@ def collate_results(raw):
             cls, attr = key
             with self.subTest(cls=cls, slotattr=attr):
                 actual = interp_results.pop(key)
-                # XXX This should not be necessary.
-                if cls == "<class 'collections.OrderedDict'>" and attr == 
'__len__':
-                    continue
                 self.assertEqual(actual, expected)
-        # XXX This should not be necessary.
-        interp_results = {k: v for k, v in interp_results.items() if k[1] != 
'__hash__'}
-        # XXX This should not be necessary.
-        interp_results.pop(("<class 'collections.OrderedDict'>", 
'__getitem__'), None)
         self.maxDiff = None
         self.assertEqual(interp_results, {})
 
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 6740da108ace91..a6483f74b7947d 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -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;

_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]

Reply via email to