From 7b299d19fda0d9173e7d8899a5226872acf7a86c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Jul 2022 16:48:47 -0600 Subject: [PATCH 01/22] Add a note about tp_cache. --- Include/cpython/object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 1ca1a576fb2329..a37842c3848734 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -217,7 +217,7 @@ struct _typeobject { inquiry tp_is_gc; /* For PyObject_IS_GC */ PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ - PyObject *tp_cache; + PyObject *tp_cache; /* no longer used */ PyObject *tp_subclasses; PyObject *tp_weaklist; destructor tp_del; From 4fefd9bda9cdb3d36c473f6048909ccbe8805e00 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Jul 2022 18:05:29 -0600 Subject: [PATCH 02/22] Hide tp_subclasses behind functions. --- Include/internal/pycore_object.h | 1 + Objects/structseq.c | 2 +- Objects/typeobject.c | 55 ++++++++++++++++++++++++-------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index c7490799d816af..1c1bfac0436cf6 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -287,6 +287,7 @@ extern int _PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, vo extern void _PyObject_ClearInstanceAttributes(PyObject *self); extern void _PyObject_FreeInstanceAttributes(PyObject *self); extern int _PyObject_IsInstanceDictEmpty(PyObject *); +extern int _PyType_HasSubclasses(PyTypeObject *); extern PyObject* _PyType_GetSubclasses(PyTypeObject *); // Access macro to the members which are floating "behind" the object diff --git a/Objects/structseq.c b/Objects/structseq.c index 24cd0e705969b2..9a7013372e688c 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -580,7 +580,7 @@ _PyStructSequence_FiniType(PyTypeObject *type) assert(type->tp_base == &PyTuple_Type); // Cannot delete a type if it still has subclasses - if (type->tp_subclasses != NULL) { + if (_PyType_HasSubclasses(type)) { return; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9eb2390118d097..7c15bc5b777cd6 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -370,6 +370,8 @@ _PyTypes_Fini(PyInterpreterState *interp) } +static PyObject * lookup_subclasses(PyTypeObject *); + void PyType_Modified(PyTypeObject *type) { @@ -392,7 +394,7 @@ PyType_Modified(PyTypeObject *type) return; } - PyObject *subclasses = type->tp_subclasses; + PyObject *subclasses = lookup_subclasses(type); if (subclasses != NULL) { assert(PyDict_CheckExact(subclasses)); @@ -778,7 +780,7 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp) Py_XDECREF(old_mro); // Avoid creating an empty list if there is no subclass - if (type->tp_subclasses != NULL) { + if (_PyType_HasSubclasses(type)) { /* Obtain a copy of subclasses list to iterate over. Otherwise type->tp_subclasses might be altered @@ -4303,7 +4305,7 @@ type_dealloc_common(PyTypeObject *type) static void clear_static_tp_subclasses(PyTypeObject *type) { - if (type->tp_subclasses == NULL) { + if (!_PyType_HasSubclasses(type)) { return; } @@ -4359,6 +4361,8 @@ _PyStaticType_Dealloc(PyTypeObject *type) } +static void clear_subclasses(PyTypeObject *self); + static void type_dealloc(PyTypeObject *type) { @@ -4378,7 +4382,7 @@ type_dealloc(PyTypeObject *type) Py_XDECREF(type->tp_bases); Py_XDECREF(type->tp_mro); Py_XDECREF(type->tp_cache); - Py_XDECREF(type->tp_subclasses); + clear_subclasses(type); /* A type's tp_doc is heap allocated, unlike the tp_doc slots * of most other objects. It's okay to cast it to char *. @@ -4398,6 +4402,18 @@ type_dealloc(PyTypeObject *type) } +static PyObject * +lookup_subclasses(PyTypeObject *self) +{ + return self->tp_subclasses; +} + +int +_PyType_HasSubclasses(PyTypeObject *self) +{ + return lookup_subclasses(self) != NULL; +} + PyObject* _PyType_GetSubclasses(PyTypeObject *self) { @@ -4406,7 +4422,7 @@ _PyType_GetSubclasses(PyTypeObject *self) return NULL; } - PyObject *subclasses = self->tp_subclasses; // borrowed ref + PyObject *subclasses = lookup_subclasses(self); // borrowed ref if (subclasses == NULL) { return list; } @@ -6784,6 +6800,22 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) } +static PyObject * +init_subclasses(PyTypeObject *self) +{ + self->tp_subclasses = PyDict_New(); + return self->tp_subclasses; +} + +static void +clear_subclasses(PyTypeObject *self) +{ + // Delete the dictionary to save memory. _PyStaticType_Dealloc() + // callers also test if tp_subclasses is NULL to check if a static type + // has no subclass. + Py_CLEAR(self->tp_subclasses); +} + static int add_subclass(PyTypeObject *base, PyTypeObject *type) { @@ -6800,9 +6832,9 @@ add_subclass(PyTypeObject *base, PyTypeObject *type) // Only get tp_subclasses after creating the key and value. // PyWeakref_NewRef() can trigger a garbage collection which can execute // arbitrary Python code and so modify base->tp_subclasses. - PyObject *subclasses = base->tp_subclasses; + PyObject *subclasses = lookup_subclasses(base); if (subclasses == NULL) { - base->tp_subclasses = subclasses = PyDict_New(); + subclasses = init_subclasses(base); if (subclasses == NULL) { Py_DECREF(key); Py_DECREF(ref); @@ -6872,7 +6904,7 @@ get_subclasses_key(PyTypeObject *type, PyTypeObject *base) static void remove_subclass(PyTypeObject *base, PyTypeObject *type) { - PyObject *subclasses = base->tp_subclasses; // borrowed ref + PyObject *subclasses = lookup_subclasses(base); // borrowed ref if (subclasses == NULL) { return; } @@ -6888,10 +6920,7 @@ remove_subclass(PyTypeObject *base, PyTypeObject *type) Py_XDECREF(key); if (PyDict_Size(subclasses) == 0) { - // Delete the dictionary to save memory. _PyStaticType_Dealloc() - // callers also test if tp_subclasses is NULL to check if a static type - // has no subclass. - Py_CLEAR(base->tp_subclasses); + clear_subclasses(base); } } @@ -8963,7 +8992,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, // It is safe to use a borrowed reference because update_subclasses() is // only used with update_slots_callback() which doesn't modify // tp_subclasses. - PyObject *subclasses = type->tp_subclasses; // borrowed ref + PyObject *subclasses = lookup_subclasses(type); // borrowed ref if (subclasses == NULL) { return 0; } From 129ca1fa02ee20a04073559179e5d829d133247a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Jul 2022 18:31:11 -0600 Subject: [PATCH 03/22] Distinquish between static and heap types. --- Objects/typeobject.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7c15bc5b777cd6..9dcfda384bb60e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4405,6 +4405,10 @@ type_dealloc(PyTypeObject *type) static PyObject * lookup_subclasses(PyTypeObject *self) { + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + return self->tp_subclasses; + } + // For static types we store them per-interpreter. return self->tp_subclasses; } @@ -6803,6 +6807,10 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) static PyObject * init_subclasses(PyTypeObject *self) { + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + self->tp_subclasses = PyDict_New(); + return self->tp_subclasses; + } self->tp_subclasses = PyDict_New(); return self->tp_subclasses; } @@ -6813,6 +6821,10 @@ clear_subclasses(PyTypeObject *self) // Delete the dictionary to save memory. _PyStaticType_Dealloc() // callers also test if tp_subclasses is NULL to check if a static type // has no subclass. + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + Py_CLEAR(self->tp_subclasses); + return; + } Py_CLEAR(self->tp_subclasses); } From 183b6b5b195a025816287753bed593350581d1a6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 7 Jul 2022 19:07:11 -0600 Subject: [PATCH 04/22] Move tp_subclasses for static types to PyInterpreterState. --- Include/cpython/object.h | 2 +- Include/internal/pycore_typeobject.h | 1 + Objects/typeobject.c | 72 +++++++++++++++++++++++----- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index a37842c3848734..4721b6224273db 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; + PyObject *tp_subclasses; /* not used for static types */ PyObject *tp_weaklist; destructor tp_del; diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index dc1c02ba412809..392831cb9d2a50 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -51,6 +51,7 @@ struct types_state { struct type_cache type_cache; size_t num_builtins_initialized; static_builtin_state builtins[_Py_MAX_STATIC_BUILTIN_TYPES]; + PyObject *subclasses; }; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9dcfda384bb60e..4b2bf9b9e1234c 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4302,10 +4302,13 @@ type_dealloc_common(PyTypeObject *type) } +static void clear_subclasses(PyTypeObject *self); + static void clear_static_tp_subclasses(PyTypeObject *type) { - if (!_PyType_HasSubclasses(type)) { + PyObject *subclasses = lookup_subclasses(type); + if (subclasses == NULL) { return; } @@ -4329,9 +4332,19 @@ clear_static_tp_subclasses(PyTypeObject *type) going to leak. This mostly only affects embedding scenarios. */ - // For now we just clear tp_subclasses. + // For now we just do a sanity check and then clear tp_subclasses. + Py_ssize_t i = 0; + PyObject *key, *ref; // borrowed ref + while (PyDict_Next(subclasses, &i, &key, &ref)) { + PyTypeObject *subclass = subclass_from_ref(ref); // borrowed + if (subclass == NULL) { + continue; + } + // All static builtin subtypes should have been finalized already. + assert(!(subclass->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + } - Py_CLEAR(type->tp_subclasses); + clear_subclasses(type); } void @@ -4361,8 +4374,6 @@ _PyStaticType_Dealloc(PyTypeObject *type) } -static void clear_subclasses(PyTypeObject *self); - static void type_dealloc(PyTypeObject *type) { @@ -4405,11 +4416,17 @@ type_dealloc(PyTypeObject *type) static PyObject * lookup_subclasses(PyTypeObject *self) { + // XXX Drop tp_subclasses? if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { return self->tp_subclasses; } // For static types we store them per-interpreter. - return self->tp_subclasses; + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->types.subclasses == NULL) { + return NULL; + } + // XXX Use PyDict_GetItemWithError()? + return PyDict_GetItem(interp->types.subclasses, (PyObject *)self); } int @@ -6811,8 +6828,24 @@ init_subclasses(PyTypeObject *self) self->tp_subclasses = PyDict_New(); return self->tp_subclasses; } - self->tp_subclasses = PyDict_New(); - return self->tp_subclasses; + /* Each interpreter has a dict mapping types to __subclasses__. + We initialize that dict (if necessary), as well as the type's dict. */ + PyObject *subclasses = PyDict_New(); + if (subclasses == NULL) { + return NULL; + } + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->types.subclasses == NULL) { + interp->types.subclasses = PyDict_New(); + if (interp->types.subclasses == NULL) { + Py_DECREF(subclasses); + return NULL; + } + } + int res = PyDict_SetItem(interp->types.subclasses, + (PyObject *)self, subclasses); + Py_DECREF(subclasses); + return res == 0 ? subclasses : NULL; } static void @@ -6825,7 +6858,17 @@ clear_subclasses(PyTypeObject *self) Py_CLEAR(self->tp_subclasses); return; } - Py_CLEAR(self->tp_subclasses); + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp->types.subclasses == NULL) { + return; + } + if (PyDict_DelItem(interp->types.subclasses, (PyObject *)self) != 0) { + PyErr_Clear(); + return; + } + if (PyDict_GET_SIZE(interp->types.subclasses) == 0) { + Py_CLEAR(interp->types.subclasses); + } } static int @@ -6903,10 +6946,13 @@ get_subclasses_key(PyTypeObject *type, PyTypeObject *base) We fall back to manually traversing the values. */ Py_ssize_t i = 0; PyObject *ref; // borrowed ref - while (PyDict_Next((PyObject *)base->tp_subclasses, &i, &key, &ref)) { - PyTypeObject *subclass = subclass_from_ref(ref); // borrowed - if (subclass == type) { - return Py_NewRef(key); + PyObject *subclasses = lookup_subclasses(base); + if (subclasses != NULL) { + while (PyDict_Next(subclasses, &i, &key, &ref)) { + PyTypeObject *subclass = subclass_from_ref(ref); // borrowed + if (subclass == type) { + return Py_NewRef(key); + } } } /* It wasn't found. */ From a707c38383b956b660e7f5031deeb3235896d23b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 13:32:16 -0600 Subject: [PATCH 05/22] Add a note about heap types and tp_subclasses. --- Objects/typeobject.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 4b2bf9b9e1234c..ee0090a3032075 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4417,6 +4417,7 @@ static PyObject * lookup_subclasses(PyTypeObject *self) { // XXX Drop tp_subclasses? + // Heap types are guaranteed to be per-interpreter. if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { return self->tp_subclasses; } @@ -6824,6 +6825,7 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) static PyObject * init_subclasses(PyTypeObject *self) { + // Heap types are guaranteed to be per-interpreter. if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { self->tp_subclasses = PyDict_New(); return self->tp_subclasses; @@ -6851,9 +6853,7 @@ init_subclasses(PyTypeObject *self) static void clear_subclasses(PyTypeObject *self) { - // Delete the dictionary to save memory. _PyStaticType_Dealloc() - // callers also test if tp_subclasses is NULL to check if a static type - // has no subclass. + // Heap types are guaranteed to be per-interpreter. if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { Py_CLEAR(self->tp_subclasses); return; @@ -6862,6 +6862,9 @@ clear_subclasses(PyTypeObject *self) if (interp->types.subclasses == NULL) { return; } + // Delete the dictionary to save memory. _PyStaticType_Dealloc() + // callers also test if tp_subclasses is NULL to check if a static type + // has no subclass. if (PyDict_DelItem(interp->types.subclasses, (PyObject *)self) != 0) { PyErr_Clear(); return; From 5dd2474366f9f3e75502ede013b59ad580613571 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 14:12:21 -0600 Subject: [PATCH 06/22] Use PyDict_GetItemWithError(). --- Objects/structseq.c | 1 + Objects/typeobject.c | 25 ++++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index 9a7013372e688c..84dd749a09db2d 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -581,6 +581,7 @@ _PyStructSequence_FiniType(PyTypeObject *type) // Cannot delete a type if it still has subclasses if (_PyType_HasSubclasses(type)) { + PyErr_Clear(); return; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ee0090a3032075..1f4ed744e4c55e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -408,6 +408,9 @@ PyType_Modified(PyTypeObject *type) PyType_Modified(subclass); } } + else if (PyErr_Occurred()) { + PyErr_Clear(); + } type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG; type->tp_version_tag = 0; /* 0 is not a valid version tag */ @@ -780,7 +783,8 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp) Py_XDECREF(old_mro); // Avoid creating an empty list if there is no subclass - if (_PyType_HasSubclasses(type)) { + res = _PyType_HasSubclasses(type); + if (res > 0) { /* Obtain a copy of subclasses list to iterate over. Otherwise type->tp_subclasses might be altered @@ -4309,6 +4313,7 @@ clear_static_tp_subclasses(PyTypeObject *type) { PyObject *subclasses = lookup_subclasses(type); if (subclasses == NULL) { + PyErr_Clear(); return; } @@ -4426,14 +4431,17 @@ lookup_subclasses(PyTypeObject *self) if (interp->types.subclasses == NULL) { return NULL; } - // XXX Use PyDict_GetItemWithError()? - return PyDict_GetItem(interp->types.subclasses, (PyObject *)self); + return PyDict_GetItemWithError(interp->types.subclasses, (PyObject *)self); } int _PyType_HasSubclasses(PyTypeObject *self) { - return lookup_subclasses(self) != NULL; + PyObject *subclasses = lookup_subclasses(self); + if (subclasses == NULL) { + return PyErr_Occurred() ? -1 : 0; + } + return 1; } PyObject* @@ -4446,6 +4454,9 @@ _PyType_GetSubclasses(PyTypeObject *self) PyObject *subclasses = lookup_subclasses(self); // borrowed ref if (subclasses == NULL) { + if (PyErr_Occurred()) { + return NULL; + } return list; } assert(PyDict_CheckExact(subclasses)); @@ -6892,6 +6903,9 @@ add_subclass(PyTypeObject *base, PyTypeObject *type) // arbitrary Python code and so modify base->tp_subclasses. PyObject *subclasses = lookup_subclasses(base); if (subclasses == NULL) { + if (PyErr_Occurred()) { + return -1; + } subclasses = init_subclasses(base); if (subclasses == NULL) { Py_DECREF(key); @@ -6967,6 +6981,7 @@ remove_subclass(PyTypeObject *base, PyTypeObject *type) { PyObject *subclasses = lookup_subclasses(base); // borrowed ref if (subclasses == NULL) { + PyErr_Clear(); return; } assert(PyDict_CheckExact(subclasses)); @@ -9055,7 +9070,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, // tp_subclasses. PyObject *subclasses = lookup_subclasses(type); // borrowed ref if (subclasses == NULL) { - return 0; + return PyErr_Occurred() ? -1 : 0; } assert(PyDict_CheckExact(subclasses)); From 80b1d26270547edef595a7ed32747bcb12f32ffb Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 15:21:35 -0600 Subject: [PATCH 07/22] Add lookup___hash__(). --- Objects/typeobject.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 1f4ed744e4c55e..948056fb7769e6 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -7969,6 +7969,18 @@ slot_tp_repr(PyObject *self) SLOT0(slot_tp_str, __str__) + +static inline PyObject * +lookup___hash__(PyObject *self, int *unbound) +{ + PyObject *func = lookup_maybe_method(self, &_Py_ID(__hash__), unbound); + if (func == Py_None) { + Py_DECREF(func); + func = NULL; + } + return func; +} + static Py_hash_t slot_tp_hash(PyObject *self) { @@ -7976,14 +7988,11 @@ slot_tp_hash(PyObject *self) Py_ssize_t h; int unbound; - func = lookup_maybe_method(self, &_Py_ID(__hash__), &unbound); - - if (func == Py_None) { - Py_DECREF(func); - func = NULL; - } - + func = lookup___hash__(self, &unbound); if (func == NULL) { + if (PyErr_Occurred()) { + return -1; + } return PyObject_HashNotImplemented(self); } From 8bced849b5ad16b8badae6f3f5295a2d304f3789 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 16:05:55 -0600 Subject: [PATCH 08/22] Handle the case of unhashable classes. --- Objects/typeobject.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 948056fb7769e6..d6d9de3edd09ca 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -149,6 +149,31 @@ static_builtin_state_clear(PyTypeObject *self) /* end static builtin helpers */ +static Py_hash_t slot_tp_hash(PyObject *self); +static inline PyObject * lookup___hash__(PyObject *self, int *unbound); + +static int +is_hashable(PyObject *obj) +{ + PyTypeObject *cls = Py_TYPE(obj); + if (cls->tp_hash != NULL && + cls->tp_hash != PyObject_HashNotImplemented) + { + if (cls->tp_hash != slot_tp_hash) { + return 1; + } + int unbound; + if (lookup___hash__((PyObject *)cls, &unbound) != NULL) { + return 1; + } + if (PyErr_Occurred()) { + return -1; + } + } + return 0; +} + + /* * finds the beginning of the docstring's introspection signature. * if present, returns a pointer pointing to the first '('. @@ -4431,7 +4456,17 @@ lookup_subclasses(PyTypeObject *self) if (interp->types.subclasses == NULL) { return NULL; } - return PyDict_GetItemWithError(interp->types.subclasses, (PyObject *)self); + PyObject *subclasses = PyDict_GetItemWithError(interp->types.subclasses, + (PyObject *)self); + // We must handle the rare case where the metaclass + // of the given type makes it unhashable. + if (subclasses == NULL && PyErr_Occurred() && + PyErr_ExceptionMatches(PyExc_TypeError) && + !is_hashable((PyObject *)self)) + { + PyErr_Clear(); + } + return subclasses; } int From 533951d1a74b498f210b3432264b93b225f2c3e9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 16:43:57 -0600 Subject: [PATCH 09/22] Really handle the case of unhashable classes. --- Objects/typeobject.c | 54 ++++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index d6d9de3edd09ca..2a392010b22495 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -173,6 +173,24 @@ is_hashable(PyObject *obj) return 0; } +static inline Py_hash_t +get_hash_with_fallback(PyObject *obj) +{ + Py_hash_t hash = PyObject_Hash(obj); + if (hash < 0) { + if (!PyErr_ExceptionMatches(PyExc_TypeError) || + is_hashable((PyObject *)obj)) + { + PyErr_WarnEx(PyExc_RuntimeWarning, "failed to hash object", 1); + } + // Fall back to object.__hash__(). + PyErr_Clear(); + hash = _Py_HashPointer(obj); + assert(!PyErr_Occurred()); + } + return hash; +} + /* * finds the beginning of the docstring's introspection signature. @@ -4456,17 +4474,11 @@ lookup_subclasses(PyTypeObject *self) if (interp->types.subclasses == NULL) { return NULL; } - PyObject *subclasses = PyDict_GetItemWithError(interp->types.subclasses, - (PyObject *)self); - // We must handle the rare case where the metaclass - // of the given type makes it unhashable. - if (subclasses == NULL && PyErr_Occurred() && - PyErr_ExceptionMatches(PyExc_TypeError) && - !is_hashable((PyObject *)self)) - { - PyErr_Clear(); - } - return subclasses; + /* We must handle the rare case where the metaclass + of the given type makes it unhashable. */ + Py_hash_t hash = get_hash_with_fallback((PyObject *)self); + return _PyDict_GetItem_KnownHash(interp->types.subclasses, + (PyObject *)self, hash); } int @@ -6890,8 +6902,11 @@ init_subclasses(PyTypeObject *self) return NULL; } } - int res = PyDict_SetItem(interp->types.subclasses, - (PyObject *)self, subclasses); + /* We must handle the rare case where the metaclass + of the given type makes it unhashable. */ + Py_hash_t hash = get_hash_with_fallback((PyObject *)self); + int res = _PyDict_SetItem_KnownHash(interp->types.subclasses, + (PyObject *)self, subclasses, hash); Py_DECREF(subclasses); return res == 0 ? subclasses : NULL; } @@ -6899,6 +6914,9 @@ init_subclasses(PyTypeObject *self) static void clear_subclasses(PyTypeObject *self) { + /* Delete the dictionary to save memory. _PyStaticType_Dealloc() + callers also test if tp_subclasses is NULL to check if a static type + has no subclass. */ // Heap types are guaranteed to be per-interpreter. if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { Py_CLEAR(self->tp_subclasses); @@ -6908,10 +6926,12 @@ clear_subclasses(PyTypeObject *self) if (interp->types.subclasses == NULL) { return; } - // Delete the dictionary to save memory. _PyStaticType_Dealloc() - // callers also test if tp_subclasses is NULL to check if a static type - // has no subclass. - if (PyDict_DelItem(interp->types.subclasses, (PyObject *)self) != 0) { + /* We must handle the rare case where the metaclass + of the given type makes it unhashable. */ + Py_hash_t hash = get_hash_with_fallback((PyObject *)self); + int res = _PyDict_DelItem_KnownHash(interp->types.subclasses, + (PyObject *)self, hash); + if (res != 0) { PyErr_Clear(); return; } From f9ab261320509fe3767471c19c1c656b9bbda03a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 17:21:06 -0600 Subject: [PATCH 10/22] Stop special-casing heap types for tp_subclasses. --- Include/cpython/object.h | 2 +- Objects/typeobject.c | 18 ------------------ 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 4721b6224273db..02e53e32abd284 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; /* not used for static types */ + PyObject *tp_subclasses; /* no longer used */ PyObject *tp_weaklist; destructor tp_del; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2a392010b22495..9fbb63ab07fac4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4464,12 +4464,6 @@ type_dealloc(PyTypeObject *type) static PyObject * lookup_subclasses(PyTypeObject *self) { - // XXX Drop tp_subclasses? - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - return self->tp_subclasses; - } - // For static types we store them per-interpreter. PyInterpreterState *interp = _PyInterpreterState_GET(); if (interp->types.subclasses == NULL) { return NULL; @@ -6883,13 +6877,6 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) static PyObject * init_subclasses(PyTypeObject *self) { - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - self->tp_subclasses = PyDict_New(); - return self->tp_subclasses; - } - /* Each interpreter has a dict mapping types to __subclasses__. - We initialize that dict (if necessary), as well as the type's dict. */ PyObject *subclasses = PyDict_New(); if (subclasses == NULL) { return NULL; @@ -6917,11 +6904,6 @@ clear_subclasses(PyTypeObject *self) /* Delete the dictionary to save memory. _PyStaticType_Dealloc() callers also test if tp_subclasses is NULL to check if a static type has no subclass. */ - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - Py_CLEAR(self->tp_subclasses); - return; - } PyInterpreterState *interp = _PyInterpreterState_GET(); if (interp->types.subclasses == NULL) { return; From ccabeb5e3d950f5398df121afb8a3e2fdc0c8ffd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 8 Jul 2022 17:22:33 -0600 Subject: [PATCH 11/22] Revert "Stop special-casing heap types for tp_subclasses." This reverts commit 0046b57446a26114fdf33d11a31a0fc84e7a764a. --- Include/cpython/object.h | 2 +- Objects/typeobject.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 02e53e32abd284..4721b6224273db 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; /* no longer used */ + PyObject *tp_subclasses; /* not used for static types */ PyObject *tp_weaklist; destructor tp_del; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9fbb63ab07fac4..12b37bfaa1b00a 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4464,6 +4464,12 @@ type_dealloc(PyTypeObject *type) static PyObject * lookup_subclasses(PyTypeObject *self) { + // XXX Drop tp_subclasses? + // Heap types are guaranteed to be per-interpreter. + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + return self->tp_subclasses; + } + // For static types we store them per-interpreter. PyInterpreterState *interp = _PyInterpreterState_GET(); if (interp->types.subclasses == NULL) { return NULL; @@ -6877,6 +6883,11 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) static PyObject * init_subclasses(PyTypeObject *self) { + // Heap types are guaranteed to be per-interpreter. + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + self->tp_subclasses = PyDict_New(); + return self->tp_subclasses; + } PyObject *subclasses = PyDict_New(); if (subclasses == NULL) { return NULL; @@ -6904,6 +6915,11 @@ clear_subclasses(PyTypeObject *self) /* Delete the dictionary to save memory. _PyStaticType_Dealloc() callers also test if tp_subclasses is NULL to check if a static type has no subclass. */ + // Heap types are guaranteed to be per-interpreter. + if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { + Py_CLEAR(self->tp_subclasses); + return; + } PyInterpreterState *interp = _PyInterpreterState_GET(); if (interp->types.subclasses == NULL) { return; From df204e867e18276ae359ec014d516109e2a6eadc Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 9 Jul 2022 12:15:11 -0600 Subject: [PATCH 12/22] Point to the relevant GH issue. --- Include/cpython/object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 4721b6224273db..6828f8ccd50e94 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; /* not used for static types */ + PyObject *tp_subclasses; /* not used for static types (see gh-94673) */ PyObject *tp_weaklist; destructor tp_del; From 3529f0f20e551a082614eec6c9712924ce3cfaf0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 9 Jul 2022 14:48:55 -0600 Subject: [PATCH 13/22] Only store builtins state on the interpreter state. --- Include/internal/pycore_typeobject.h | 2 +- Objects/typeobject.c | 140 ++++++--------------------- 2 files changed, 28 insertions(+), 114 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 392831cb9d2a50..f708da246a0a5d 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -45,13 +45,13 @@ struct type_cache { typedef struct { PyTypeObject *type; + PyObject *subclasses; } static_builtin_state; struct types_state { struct type_cache type_cache; size_t num_builtins_initialized; static_builtin_state builtins[_Py_MAX_STATIC_BUILTIN_TYPES]; - PyObject *subclasses; }; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 12b37bfaa1b00a..86f53b402a5876 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -138,6 +138,7 @@ static_builtin_state_clear(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = NULL; + /* state->subclasses is left NULL until init_subclasses() sets it. */ static_builtin_index_clear(self); assert(interp->types.num_builtins_initialized > 0); @@ -149,49 +150,6 @@ static_builtin_state_clear(PyTypeObject *self) /* end static builtin helpers */ -static Py_hash_t slot_tp_hash(PyObject *self); -static inline PyObject * lookup___hash__(PyObject *self, int *unbound); - -static int -is_hashable(PyObject *obj) -{ - PyTypeObject *cls = Py_TYPE(obj); - if (cls->tp_hash != NULL && - cls->tp_hash != PyObject_HashNotImplemented) - { - if (cls->tp_hash != slot_tp_hash) { - return 1; - } - int unbound; - if (lookup___hash__((PyObject *)cls, &unbound) != NULL) { - return 1; - } - if (PyErr_Occurred()) { - return -1; - } - } - return 0; -} - -static inline Py_hash_t -get_hash_with_fallback(PyObject *obj) -{ - Py_hash_t hash = PyObject_Hash(obj); - if (hash < 0) { - if (!PyErr_ExceptionMatches(PyExc_TypeError) || - is_hashable((PyObject *)obj)) - { - PyErr_WarnEx(PyExc_RuntimeWarning, "failed to hash object", 1); - } - // Fall back to object.__hash__(). - PyErr_Clear(); - hash = _Py_HashPointer(obj); - assert(!PyErr_Occurred()); - } - return hash; -} - - /* * finds the beginning of the docstring's introspection signature. * if present, returns a pointer pointing to the first '('. @@ -4464,28 +4422,22 @@ type_dealloc(PyTypeObject *type) static PyObject * lookup_subclasses(PyTypeObject *self) { - // XXX Drop tp_subclasses? - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - return self->tp_subclasses; - } - // For static types we store them per-interpreter. - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (interp->types.subclasses == NULL) { - return NULL; + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + static_builtin_state *state = _PyStaticType_GetState(self); + assert(state != NULL); + return state->subclasses; } - /* We must handle the rare case where the metaclass - of the given type makes it unhashable. */ - Py_hash_t hash = get_hash_with_fallback((PyObject *)self); - return _PyDict_GetItem_KnownHash(interp->types.subclasses, - (PyObject *)self, hash); + return self->tp_subclasses; } int _PyType_HasSubclasses(PyTypeObject *self) { - PyObject *subclasses = lookup_subclasses(self); - if (subclasses == NULL) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN && + _PyStaticType_GetState(self) == NULL) { + return 0; + } + if (lookup_subclasses(self) == NULL) { return PyErr_Occurred() ? -1 : 0; } return 1; @@ -6883,30 +6835,16 @@ _PyStaticType_InitBuiltin(PyTypeObject *self) static PyObject * init_subclasses(PyTypeObject *self) { - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - self->tp_subclasses = PyDict_New(); - return self->tp_subclasses; - } PyObject *subclasses = PyDict_New(); if (subclasses == NULL) { return NULL; } - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (interp->types.subclasses == NULL) { - interp->types.subclasses = PyDict_New(); - if (interp->types.subclasses == NULL) { - Py_DECREF(subclasses); - return NULL; - } + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + static_builtin_state *state = _PyStaticType_GetState(self); + state->subclasses = subclasses; } - /* We must handle the rare case where the metaclass - of the given type makes it unhashable. */ - Py_hash_t hash = get_hash_with_fallback((PyObject *)self); - int res = _PyDict_SetItem_KnownHash(interp->types.subclasses, - (PyObject *)self, subclasses, hash); - Py_DECREF(subclasses); - return res == 0 ? subclasses : NULL; + self->tp_subclasses = subclasses; + return subclasses; } static void @@ -6915,27 +6853,12 @@ clear_subclasses(PyTypeObject *self) /* Delete the dictionary to save memory. _PyStaticType_Dealloc() callers also test if tp_subclasses is NULL to check if a static type has no subclass. */ - // Heap types are guaranteed to be per-interpreter. - if (self->tp_flags & Py_TPFLAGS_HEAPTYPE) { - Py_CLEAR(self->tp_subclasses); - return; - } - PyInterpreterState *interp = _PyInterpreterState_GET(); - if (interp->types.subclasses == NULL) { - return; - } - /* We must handle the rare case where the metaclass - of the given type makes it unhashable. */ - Py_hash_t hash = get_hash_with_fallback((PyObject *)self); - int res = _PyDict_DelItem_KnownHash(interp->types.subclasses, - (PyObject *)self, hash); - if (res != 0) { - PyErr_Clear(); + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + static_builtin_state *state = _PyStaticType_GetState(self); + Py_CLEAR(state->subclasses); return; } - if (PyDict_GET_SIZE(interp->types.subclasses) == 0) { - Py_CLEAR(interp->types.subclasses); - } + Py_CLEAR(self->tp_subclasses); } static int @@ -8022,18 +7945,6 @@ slot_tp_repr(PyObject *self) SLOT0(slot_tp_str, __str__) - -static inline PyObject * -lookup___hash__(PyObject *self, int *unbound) -{ - PyObject *func = lookup_maybe_method(self, &_Py_ID(__hash__), unbound); - if (func == Py_None) { - Py_DECREF(func); - func = NULL; - } - return func; -} - static Py_hash_t slot_tp_hash(PyObject *self) { @@ -8041,11 +7952,14 @@ slot_tp_hash(PyObject *self) Py_ssize_t h; int unbound; - func = lookup___hash__(self, &unbound); + func = lookup_maybe_method(self, &_Py_ID(__hash__), &unbound); + + if (func == Py_None) { + Py_DECREF(func); + func = NULL; + } + if (func == NULL) { - if (PyErr_Occurred()) { - return -1; - } return PyObject_HashNotImplemented(self); } From 47c6ded6f736a840747a953a6ecfb817b06e732e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 19 Jul 2022 09:55:35 -0600 Subject: [PATCH 14/22] state.subclasses -> state.tp_subclasses --- Include/internal/pycore_typeobject.h | 2 +- Objects/typeobject.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index f708da246a0a5d..7bf622f3664c7c 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -45,7 +45,7 @@ struct type_cache { typedef struct { PyTypeObject *type; - PyObject *subclasses; + PyObject *tp_subclasses; } static_builtin_state; struct types_state { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 86f53b402a5876..adace6762776d7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -127,6 +127,7 @@ static_builtin_state_init(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = self; + /* state->tp_subclasses is left NULL until init_subclasses() sets it. */ } static void @@ -138,7 +139,6 @@ static_builtin_state_clear(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = NULL; - /* state->subclasses is left NULL until init_subclasses() sets it. */ static_builtin_index_clear(self); assert(interp->types.num_builtins_initialized > 0); @@ -4425,7 +4425,7 @@ lookup_subclasses(PyTypeObject *self) if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { static_builtin_state *state = _PyStaticType_GetState(self); assert(state != NULL); - return state->subclasses; + return state->tp_subclasses; } return self->tp_subclasses; } @@ -6841,7 +6841,7 @@ init_subclasses(PyTypeObject *self) } if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { static_builtin_state *state = _PyStaticType_GetState(self); - state->subclasses = subclasses; + state->tp_subclasses = subclasses; } self->tp_subclasses = subclasses; return subclasses; @@ -6855,7 +6855,7 @@ clear_subclasses(PyTypeObject *self) has no subclass. */ if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { static_builtin_state *state = _PyStaticType_GetState(self); - Py_CLEAR(state->subclasses); + Py_CLEAR(state->tp_subclasses); return; } Py_CLEAR(self->tp_subclasses); From eade37b4ff14c8ad6a9a94d6b96fac88670782bd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 19 Jul 2022 10:01:29 -0600 Subject: [PATCH 15/22] Do not set type.tp_subclasses for static builtin types. --- Objects/typeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index adace6762776d7..ee9362964f9512 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6842,6 +6842,7 @@ init_subclasses(PyTypeObject *self) if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { static_builtin_state *state = _PyStaticType_GetState(self); state->tp_subclasses = subclasses; + return subclasses; } self->tp_subclasses = subclasses; return subclasses; From 5687c8ebb64f9024ce07161710c561c64ee58776 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 25 Jul 2022 17:10:47 -0600 Subject: [PATCH 16/22] Store the index in tp_subclasses. --- Include/cpython/object.h | 3 +-- Objects/typeobject.c | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 6828f8ccd50e94..40ff22faf611a9 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; /* not used for static types (see gh-94673) */ + PyObject *tp_subclasses; /* for static builtin types this is an index */ PyObject *tp_weaklist; destructor tp_del; @@ -227,7 +227,6 @@ struct _typeobject { destructor tp_finalize; vectorcallfunc tp_vectorcall; - size_t tp_static_builtin_index; /* 0 means "not initialized" */ }; /* This struct is used by the specializer diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ee9362964f9512..7115b2a6860803 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -73,7 +73,7 @@ static inline PyTypeObject * subclass_from_ref(PyObject *ref); static inline int static_builtin_index_is_set(PyTypeObject *self) { - return self->tp_static_builtin_index > 0; + return self->tp_subclasses != NULL; } static inline size_t @@ -81,7 +81,7 @@ static_builtin_index_get(PyTypeObject *self) { assert(static_builtin_index_is_set(self)); /* We store a 1-based index so 0 can mean "not initialized". */ - return self->tp_static_builtin_index - 1; + return (size_t)self->tp_subclasses - 1; } static inline void @@ -89,13 +89,13 @@ static_builtin_index_set(PyTypeObject *self, size_t index) { assert(index < _Py_MAX_STATIC_BUILTIN_TYPES); /* We store a 1-based index so 0 can mean "not initialized". */ - self->tp_static_builtin_index = index + 1; + self->tp_subclasses = (PyObject *)(index + 1); } static inline void static_builtin_index_clear(PyTypeObject *self) { - self->tp_static_builtin_index = 0; + self->tp_subclasses = NULL; } static inline static_builtin_state * From 2fa815ead60546f246a44bf2071634127b00a33d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 26 Jul 2022 18:20:13 -0600 Subject: [PATCH 17/22] Fix test_sys. --- Lib/test/test_sys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index f629dd5f034728..1dc10d8b0a39ac 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1507,7 +1507,7 @@ def delx(self): del self.__x check((1,2,3), vsize('') + 3*self.P) # type # static type: PyTypeObject - fmt = 'P2nPI13Pl4Pn9Pn12PIPI' + fmt = 'P2nPI13Pl4Pn9Pn12PIP' s = vsize('2P' + fmt) check(int, s) # class From 62a48da76fe5772abec815ae0e3337a572036e66 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 28 Jul 2022 18:27:39 -0600 Subject: [PATCH 18/22] Document the change. --- Doc/c-api/typeobj.rst | 9 ++++++++- Doc/whatsnew/3.12.rst | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index a331e9c1885092..86362df34efb7c 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -1930,7 +1930,14 @@ and :c:type:`PyType_Type` effectively act as defaults.) .. c:member:: PyObject* PyTypeObject.tp_subclasses - List of weak references to subclasses. Internal use only. + The collection of weak references to subclasses. Internal use only. + + .. versionchanged:: 3.12 + + Internals detail: For the static builtin types this field no longer + holds the subclasses. Those are now stored on ``PyInterpreterState``. + This field is re-purposed to hold the index into the type's storage + on each interpreter state. **Inheritance:** diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 03020559362b1d..98076dbf018e59 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -413,6 +413,14 @@ Porting to Python 3.12 ``Py_UNICODE*`` based format (e.g. ``u``, ``Z``) anymore. Please migrate to other formats for Unicode like ``s``, ``z``, ``es``, and ``U``. +* ``tp_subclasses`` is no longer used for any static builtin types. + The subclasses are stored internally elsewhere. However, ``tp_subclasses`` + may still hold data that will cause a crash if used as an object pointer. + This internal-only ``PyTypeObject`` field should not be used. Use the + exist public C-API or Python API to access ``__subclasses__`` instead. + We mention this in case anyone someone happens to be accessing the + field directly anyway. + Deprecated ---------- From 12d682a85caea30ede22943b2e6feda8a9dfb34d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 28 Jul 2022 18:38:34 -0600 Subject: [PATCH 19/22] Make tp_subclasses void*. --- Doc/c-api/typeobj.rst | 6 +++--- Include/cpython/object.h | 2 +- Objects/typeobject.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index 86362df34efb7c..b5733a4cfeaca8 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -135,7 +135,7 @@ Quick Reference +------------------------------------------------+-----------------------------------+-------------------+---+---+---+---+ | [:c:member:`~PyTypeObject.tp_cache`] | :c:type:`PyObject` * | | | | | +------------------------------------------------+-----------------------------------+-------------------+---+---+---+---+ - | [:c:member:`~PyTypeObject.tp_subclasses`] | :c:type:`PyObject` * | __subclasses__ | | | | + | [:c:member:`~PyTypeObject.tp_subclasses`] | void * | __subclasses__ | | | | +------------------------------------------------+-----------------------------------+-------------------+---+---+---+---+ | [:c:member:`~PyTypeObject.tp_weaklist`] | :c:type:`PyObject` * | | | | | +------------------------------------------------+-----------------------------------+-------------------+---+---+---+---+ @@ -1936,8 +1936,8 @@ and :c:type:`PyType_Type` effectively act as defaults.) Internals detail: For the static builtin types this field no longer holds the subclasses. Those are now stored on ``PyInterpreterState``. - This field is re-purposed to hold the index into the type's storage - on each interpreter state. + For static builtin types, this field is re-purposed to hold the index + into the type's storage on each interpreter state. **Inheritance:** diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 40ff22faf611a9..e42cc8208a86e9 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -218,7 +218,7 @@ struct _typeobject { PyObject *tp_bases; PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; /* no longer used */ - PyObject *tp_subclasses; /* for static builtin types this is an index */ + void *tp_subclasses; /* for static builtin types this is an index */ PyObject *tp_weaklist; destructor tp_del; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7115b2a6860803..9702d7d606949a 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4427,7 +4427,7 @@ lookup_subclasses(PyTypeObject *self) assert(state != NULL); return state->tp_subclasses; } - return self->tp_subclasses; + return (PyObject *)self->tp_subclasses; } int @@ -6844,7 +6844,7 @@ init_subclasses(PyTypeObject *self) state->tp_subclasses = subclasses; return subclasses; } - self->tp_subclasses = subclasses; + self->tp_subclasses = (void *)subclasses; return subclasses; } From dace9c485edb66be76e4af501bebcf58c3479869 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 28 Jul 2022 18:45:59 -0600 Subject: [PATCH 20/22] lookup_subclasses() never fails. --- Objects/structseq.c | 1 - Objects/typeobject.c | 15 +++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/Objects/structseq.c b/Objects/structseq.c index 84dd749a09db2d..9a7013372e688c 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -581,7 +581,6 @@ _PyStructSequence_FiniType(PyTypeObject *type) // Cannot delete a type if it still has subclasses if (_PyType_HasSubclasses(type)) { - PyErr_Clear(); return; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9702d7d606949a..96999f544c9754 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -784,8 +784,7 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp) Py_XDECREF(old_mro); // Avoid creating an empty list if there is no subclass - res = _PyType_HasSubclasses(type); - if (res > 0) { + if (_PyType_HasSubclasses(type)) { /* Obtain a copy of subclasses list to iterate over. Otherwise type->tp_subclasses might be altered @@ -4314,7 +4313,6 @@ clear_static_tp_subclasses(PyTypeObject *type) { PyObject *subclasses = lookup_subclasses(type); if (subclasses == NULL) { - PyErr_Clear(); return; } @@ -4438,7 +4436,7 @@ _PyType_HasSubclasses(PyTypeObject *self) return 0; } if (lookup_subclasses(self) == NULL) { - return PyErr_Occurred() ? -1 : 0; + return 0; } return 1; } @@ -4453,9 +4451,6 @@ _PyType_GetSubclasses(PyTypeObject *self) PyObject *subclasses = lookup_subclasses(self); // borrowed ref if (subclasses == NULL) { - if (PyErr_Occurred()) { - return NULL; - } return list; } assert(PyDict_CheckExact(subclasses)); @@ -6880,9 +6875,6 @@ add_subclass(PyTypeObject *base, PyTypeObject *type) // arbitrary Python code and so modify base->tp_subclasses. PyObject *subclasses = lookup_subclasses(base); if (subclasses == NULL) { - if (PyErr_Occurred()) { - return -1; - } subclasses = init_subclasses(base); if (subclasses == NULL) { Py_DECREF(key); @@ -6958,7 +6950,6 @@ remove_subclass(PyTypeObject *base, PyTypeObject *type) { PyObject *subclasses = lookup_subclasses(base); // borrowed ref if (subclasses == NULL) { - PyErr_Clear(); return; } assert(PyDict_CheckExact(subclasses)); @@ -9047,7 +9038,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, // tp_subclasses. PyObject *subclasses = lookup_subclasses(type); // borrowed ref if (subclasses == NULL) { - return PyErr_Occurred() ? -1 : 0; + return 0; } assert(PyDict_CheckExact(subclasses)); From e7f580d7d9c565497a96f76f80939cf81d3e1105 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 29 Jul 2022 12:45:24 +0200 Subject: [PATCH 21/22] Adjust docs for ``tp_subclasses`` - Member documentation describes the current state, for users. - The ``versionchanged`` note describes the change. - The What's New entry is, ideally, usable for someone stuck with maintaining a mysterious legacy codebase that stopped working. Details of internals are left out -- if it isn't enough to look at the source, they should be documented in the devguide rather than user-facing docs. --- Doc/c-api/typeobj.rst | 13 +++++++------ Doc/whatsnew/3.12.rst | 16 +++++++++------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index af6fd2803e8d9d..eab0883636c060 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -1928,16 +1928,17 @@ and :c:type:`PyType_Type` effectively act as defaults.) This field is not inherited. -.. c:member:: PyObject* PyTypeObject.tp_subclasses +.. c:member:: void* PyTypeObject.tp_subclasses - The collection of weak references to subclasses. Internal use only. + A collection of subclasses. Internal use only. May be an invalid pointer. + + To get a list of subclasses, call the Python method + :py:meth:`~class.__subclasses__`. .. versionchanged:: 3.12 - Internals detail: For the static builtin types this field no longer - holds the subclasses. Those are now stored on ``PyInterpreterState``. - For static builtin types, this field is re-purposed to hold the index - into the type's storage on each interpreter state. + For some types, this field does not hold a valid :c:expr:`PyObject*`. + The type was changed to :c:expr:`void*` to indicate this. **Inheritance:** diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 7b0921a8d02f2d..f7c4b531c6b21b 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -420,13 +420,15 @@ Porting to Python 3.12 using the existing public C-API instead, or, if necessary, the (internal-only) ``_PyObject_GET_WEAKREFS_LISTPTR()`` macro. -* ``tp_subclasses`` is no longer used for any static builtin types. - The subclasses are stored internally elsewhere. However, ``tp_subclasses`` - may still hold data that will cause a crash if used as an object pointer. - This internal-only ``PyTypeObject`` field should not be used. Use the - exist public C-API or Python API to access ``__subclasses__`` instead. - We mention this in case anyone someone happens to be accessing the - field directly anyway. +* This internal-only :c:member:`PyTypeObject.tp_subclasses` may now not be + a valid object pointer. Its type was changed to :c:expr:`void *` to + reflect this. We mention this in case someone happens to be accessing the + internal-only field directly. + + To get a list of subclasses, call the Python method + :py:meth:`~class.__subclasses__` (using :c:func:`PyObject_CallMethod`, + for example). + Deprecated ---------- From 6b1976365329161b62bf9d14428526e721f09536 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 4 Aug 2022 11:37:31 -0600 Subject: [PATCH 22/22] lookup_subclasses() never fails. --- Objects/typeobject.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 41c3f87ba3e8c4..823b852a0f65f9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -412,9 +412,6 @@ PyType_Modified(PyTypeObject *type) PyType_Modified(subclass); } } - else if (PyErr_Occurred()) { - PyErr_Clear(); - } type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG; type->tp_version_tag = 0; /* 0 is not a valid version tag */