From 67c8a97affcb9dbb12f44fdb7767e756b82ea34b Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Tue, 5 Oct 2021 16:40:36 -0500 Subject: [PATCH 01/18] BUG: Inherit MetaClass from bases in FromSpec API This checks the bases of of a type created using the FromSpec API to inherit the bases metaclasses. The MetaClasses alloc function will be called as is done in `tp_new` for classes created in Python. --- Modules/_testcapimodule.c | 142 ++++++++++++++++++++++++++++++++++++++ Objects/typeobject.c | 68 ++++++++++-------- 2 files changed, 181 insertions(+), 29 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 51323f03e23689..7523e0f5bd6008 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1173,6 +1173,143 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored)) } +/* + * Small helper to import abc.ABC and ctypes.Array for testing. Both + * are (incompatible) MetaClass instances. If Array is NULL it is not filled. + */ +static int +import_abc_and_array(PyObject **ABC, PyObject **Array) +{ + PyObject *abc_mod = PyImport_ImportModule("abc"); + if (abc_mod == NULL) { + return -1; + } + *ABC = PyObject_GetAttrString(abc_mod, "ABC"); + Py_DECREF(abc_mod); + if (*ABC == NULL) { + return -1; + } + if (Array == NULL) { + return 0; + } + + PyObject *ctypes_mod = PyImport_ImportModule("ctypes"); + if (ctypes_mod == NULL) { + Py_CLEAR(*ABC); + return -1; + } + *Array = PyObject_GetAttrString(ctypes_mod, "Array"); + Py_DECREF(ctypes_mod); + if (*Array == NULL) { + Py_CLEAR(*ABC); + return -1; + } + return 0; +} + + +static PyType_Slot MinimalType_slots[] = { + {0, 0}, +}; + +static PyType_Spec MinimalType_spec = { + "_testcapi.MinimalSpecType", + 0, + 0, + Py_TPFLAGS_DEFAULT, + MinimalType_slots +}; + + +static PyObject * +test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + /* Get two (incompatible) MetaTypes */ + PyObject *ABC; + if (import_abc_and_array(&ABC, NULL) < 0) { + return NULL; + } + + PyObject *bases = PyTuple_Pack(1, ABC); + if (bases == NULL) { + Py_DECREF(ABC); + return NULL; + } + PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases); + Py_DECREF(bases); + if (new == NULL) { + Py_DECREF(ABC); + return NULL; + } + if (Py_TYPE(new) != Py_TYPE(ABC)) { + PyErr_SetString(PyExc_AssertionError, + "MetaType appears not correctly inherited from ABC!"); + Py_DECREF(ABC); + Py_DECREF(new); + return NULL; + } + Py_DECREF(ABC); + Py_DECREF(new); + Py_RETURN_NONE; +} + + +static PyObject * +test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + /* Get two (incompatible) MetaTypes */ + PyObject *ABC, *Array; + + if (import_abc_and_array(&ABC, &Array) < 0) { + return NULL; + } + + PyObject *bases = PyTuple_Pack(2, ABC, Array); + Py_DECREF(ABC); + Py_DECREF(Array); + if (bases == NULL) { + return NULL; + } + /* + * The following should raise a TypeError due to a MetaClass conflict. + */ + PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases); + Py_DECREF(bases); + if (new != NULL) { + Py_DECREF(new); + PyErr_SetString(PyExc_AssertionError, + "MetaType conflict not recognized by PyType_FromSpecWithBases"); + return NULL; + } + if (PyErr_ExceptionMatches(PyExc_TypeError)) { + PyObject *type, *value, *traceback, *meta_error_string; + + PyErr_Fetch(&type, &value, &traceback); + Py_DECREF(type); + Py_XDECREF(traceback); + + meta_error_string = PyUnicode_FromString("metaclass conflict:"); + if (meta_error_string == NULL) { + Py_DECREF(value); + return NULL; + } + int res = PyUnicode_Contains(value, meta_error_string); + Py_DECREF(value); + Py_DECREF(meta_error_string); + if (res < 0) { + return NULL; + } + if (res == 0) { + PyErr_SetString(PyExc_AssertionError, + "TypeError did not inlclude expected message."); + return NULL; + } + Py_RETURN_NONE; + } + return NULL; +} + + static PyObject * test_get_type_qualname(PyObject *self, PyObject *Py_UNUSED(ignored)) { @@ -5722,6 +5859,11 @@ static PyMethodDef TestMethods[] = { {"get_args", get_args, METH_VARARGS}, {"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS}, {"test_get_type_name", test_get_type_name, METH_NOARGS}, + {"test_from_spec_metatype_inheritance", test_from_spec_metatype_inheritance, + METH_NOARGS}, + {"test_from_spec_invalid_metatype_inheritance", + test_from_spec_invalid_metatype_inheritance, + METH_NOARGS}, {"test_get_type_qualname", test_get_type_qualname, METH_NOARGS}, {"get_kwargs", (PyCFunction)(void(*)(void))get_kwargs, METH_VARARGS|METH_KEYWORDS}, diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ec274a025de30c..845e321bd455b0 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3398,37 +3398,12 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) } } - res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nmembers); - if (res == NULL) - return NULL; - res_start = (char*)res; - if (spec->name == NULL) { PyErr_SetString(PyExc_SystemError, "Type spec does not define the name field."); - goto fail; + return NULL; } - /* Set the type name and qualname */ - const char *s = strrchr(spec->name, '.'); - if (s == NULL) - s = spec->name; - else - s++; - - type = &res->ht_type; - /* The flags must be initialized early, before the GC traverses us */ - type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE; - res->ht_name = PyUnicode_FromString(s); - if (!res->ht_name) - goto fail; - res->ht_qualname = res->ht_name; - Py_INCREF(res->ht_qualname); - type->tp_name = spec->name; - - Py_XINCREF(module); - res->ht_module = module; - /* Adjust for empty tuple bases */ if (!bases) { base = &PyBaseObject_Type; @@ -3443,11 +3418,11 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) if (!bases) { bases = PyTuple_Pack(1, base); if (!bases) - goto fail; + return NULL; } else if (!PyTuple_Check(bases)) { PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple"); - goto fail; + return NULL; } else { Py_INCREF(bases); @@ -3456,12 +3431,47 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) else if (!PyTuple_Check(bases)) { bases = PyTuple_Pack(1, bases); if (!bases) - goto fail; + return NULL; } else { Py_INCREF(bases); } + /* NOTE: Missing API to replace `&PyType_Type` below, see bpo-15870 */ + PyTypeObject *metatype = _PyType_CalculateMetaclass(&PyType_Type, bases); + if (metatype == NULL) { + Py_DECREF(bases); + return NULL; + } + res = (PyHeapTypeObject*)metatype->tp_alloc(metatype, nmembers); + if (res == NULL) { + Py_DECREF(bases); + return NULL; + } + res_start = (char*)res; + + /* Set the type name and qualname */ + const char *s = strrchr(spec->name, '.'); + if (s == NULL) + s = spec->name; + else + s++; + + type = &res->ht_type; + /* The flags must be initialized early, before the GC traverses us */ + type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE; + res->ht_name = PyUnicode_FromString(s); + if (!res->ht_name) { + Py_DECREF(bases); + goto fail; + } + res->ht_qualname = res->ht_name; + Py_INCREF(res->ht_qualname); + type->tp_name = spec->name; + + Py_XINCREF(module); + res->ht_module = module; + /* Calculate best base, and check that all bases are type objects */ base = best_base(bases); if (base == NULL) { From 2012aaa637fc4eadc941424782223bf762d018be Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 5 Oct 2021 21:59:44 +0000 Subject: [PATCH 02/18] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst diff --git a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst new file mode 100644 index 00000000000000..3e9f6f484d8d89 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst @@ -0,0 +1 @@ +The `PyType_FromSpec` API will now find the correct MetaClass from the provided bases and call the ``alloc`` function of the MetaType. An error will be raised if there is a MetaClass conflict. \ No newline at end of file From 881ba13faf8e725947a5a488aec68316c7b19cc1 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Tue, 19 Oct 2021 19:10:21 -0500 Subject: [PATCH 03/18] MAINT: Address review comments First, use `goto fail` unconditionally, and second try to simplify the tests by relying only on ctypes private (MetaClass using) API. --- Modules/_testcapimodule.c | 69 ++++++--------------------------------- Objects/typeobject.c | 17 +++++----- 2 files changed, 18 insertions(+), 68 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 7523e0f5bd6008..fbc551989a50c8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -24,6 +24,9 @@ #include #include +#include /* required by ctypes.h */ +#include "_ctypes/ctypes.h" /* To test metaclass inheritance */ + #ifdef MS_WINDOWS # include /* struct timeval */ #endif @@ -1173,48 +1176,13 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored)) } -/* - * Small helper to import abc.ABC and ctypes.Array for testing. Both - * are (incompatible) MetaClass instances. If Array is NULL it is not filled. - */ -static int -import_abc_and_array(PyObject **ABC, PyObject **Array) -{ - PyObject *abc_mod = PyImport_ImportModule("abc"); - if (abc_mod == NULL) { - return -1; - } - *ABC = PyObject_GetAttrString(abc_mod, "ABC"); - Py_DECREF(abc_mod); - if (*ABC == NULL) { - return -1; - } - if (Array == NULL) { - return 0; - } - - PyObject *ctypes_mod = PyImport_ImportModule("ctypes"); - if (ctypes_mod == NULL) { - Py_CLEAR(*ABC); - return -1; - } - *Array = PyObject_GetAttrString(ctypes_mod, "Array"); - Py_DECREF(ctypes_mod); - if (*Array == NULL) { - Py_CLEAR(*ABC); - return -1; - } - return 0; -} - - static PyType_Slot MinimalType_slots[] = { {0, 0}, }; static PyType_Spec MinimalType_spec = { "_testcapi.MinimalSpecType", - 0, + sizeof(CDataObject), 0, Py_TPFLAGS_DEFAULT, MinimalType_slots @@ -1224,31 +1192,22 @@ static PyType_Spec MinimalType_spec = { static PyObject * test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) { - /* Get two (incompatible) MetaTypes */ - PyObject *ABC; - if (import_abc_and_array(&ABC, NULL) < 0) { - return NULL; - } - - PyObject *bases = PyTuple_Pack(1, ABC); + PyObject *bases = PyTuple_Pack(1, PyCArray_Type); if (bases == NULL) { - Py_DECREF(ABC); return NULL; } + PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases); Py_DECREF(bases); if (new == NULL) { - Py_DECREF(ABC); return NULL; } - if (Py_TYPE(new) != Py_TYPE(ABC)) { + if (Py_TYPE(new) != Py_TYPE(&PyCArray_Type)) { PyErr_SetString(PyExc_AssertionError, "MetaType appears not correctly inherited from ABC!"); - Py_DECREF(ABC); Py_DECREF(new); return NULL; } - Py_DECREF(ABC); Py_DECREF(new); Py_RETURN_NONE; } @@ -1257,19 +1216,11 @@ test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored) static PyObject * test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) { - /* Get two (incompatible) MetaTypes */ - PyObject *ABC, *Array; - - if (import_abc_and_array(&ABC, &Array) < 0) { - return NULL; - } - - PyObject *bases = PyTuple_Pack(2, ABC, Array); - Py_DECREF(ABC); - Py_DECREF(Array); + PyObject *bases = PyTuple_Pack(2, PyCArray_Type, PyCFuncPtr_Type); if (bases == NULL) { return NULL; } + /* * The following should raise a TypeError due to a MetaClass conflict. */ @@ -1286,7 +1237,7 @@ test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED( PyErr_Fetch(&type, &value, &traceback); Py_DECREF(type); - Py_XDECREF(traceback); + Py_DECREF(traceback); meta_error_string = PyUnicode_FromString("metaclass conflict:"); if (meta_error_string == NULL) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 845e321bd455b0..a01da6755a4c1d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3360,7 +3360,7 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) PyObject * PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) { - PyHeapTypeObject *res; + PyHeapTypeObject *res = NULL; PyObject *modname; PyTypeObject *type, *base; int r; @@ -3401,7 +3401,7 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) if (spec->name == NULL) { PyErr_SetString(PyExc_SystemError, "Type spec does not define the name field."); - return NULL; + goto fail; } /* Adjust for empty tuple bases */ @@ -3418,11 +3418,11 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) if (!bases) { bases = PyTuple_Pack(1, base); if (!bases) - return NULL; + goto fail; } else if (!PyTuple_Check(bases)) { PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple"); - return NULL; + goto fail; } else { Py_INCREF(bases); @@ -3431,22 +3431,21 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) else if (!PyTuple_Check(bases)) { bases = PyTuple_Pack(1, bases); if (!bases) - return NULL; + goto fail; } else { Py_INCREF(bases); } - /* NOTE: Missing API to replace `&PyType_Type` below, see bpo-15870 */ PyTypeObject *metatype = _PyType_CalculateMetaclass(&PyType_Type, bases); if (metatype == NULL) { Py_DECREF(bases); - return NULL; + goto fail; } res = (PyHeapTypeObject*)metatype->tp_alloc(metatype, nmembers); if (res == NULL) { Py_DECREF(bases); - return NULL; + goto fail; } res_start = (char*)res; @@ -3614,7 +3613,7 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) return (PyObject*)res; fail: - Py_DECREF(res); + Py_XDECREF(res); return NULL; } From aad745657bd69039ea906286a5bacf38174f95cc Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 27 May 2022 13:36:05 +0200 Subject: [PATCH 04/18] Disallow non-type metaclasses --- Objects/typeobject.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index adc6a45001dc33..14ae66b2ce8ae0 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3455,6 +3455,13 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, Py_DECREF(bases); goto fail; } + if (!PyType_Check(metaclass)) { + Py_DECREF(bases); + PyErr_Format(PyExc_TypeError, + "Metaclass '%R' is not a subclass of 'type'.", + metaclass); + goto fail; + } if (metaclass->tp_new != PyType_Type.tp_new) { Py_DECREF(bases); PyErr_SetString(PyExc_TypeError, From aba4208e975802acb910714f91fe7ee4883fe9a0 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 27 May 2022 16:47:16 +0200 Subject: [PATCH 05/18] Use custom metaclasses for the tests, rather than stuff from ctypes --- Modules/_testcapimodule.c | 129 ++++++++++++++++++++++++++------------ 1 file changed, 90 insertions(+), 39 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index b03123e11e6e81..125e7b1aa6320b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1211,88 +1211,139 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored)) } -static PyType_Slot MinimalType_slots[] = { +static PyType_Slot empty_type_slots[] = { {0, 0}, }; -static PyType_Spec MinimalType_spec = { - "_testcapi.MinimalSpecType", - sizeof(CDataObject), - 0, - Py_TPFLAGS_DEFAULT, - MinimalType_slots +static PyType_Spec MinimalMetaclass_spec = { + .name = "_testcapi.MinimalMetaclass", + .basicsize = sizeof(PyHeapTypeObject), + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .slots = empty_type_slots, }; +static PyType_Spec MinimalType_spec = { + .name = "_testcapi.MinimalSpecType", + .basicsize = sizeof(PyObject), + .flags = Py_TPFLAGS_DEFAULT, + .slots = empty_type_slots, +}; static PyObject * test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject *bases = PyTuple_Pack(1, PyCArray_Type); - if (bases == NULL) { - return NULL; + PyObject *metaclass = NULL; + PyObject *class = NULL; + PyObject *new = NULL; + PyObject *result = NULL; + + metaclass = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type); + if (metaclass == NULL) { + goto finally; + } + class = PyObject_CallFunction(metaclass, "s(){}", "TestClass"); + if (class == NULL) { + goto finally; } - PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases); - Py_DECREF(bases); + new = PyType_FromSpecWithBases(&MinimalType_spec, class); if (new == NULL) { - return NULL; + goto finally; } - if (Py_TYPE(new) != Py_TYPE(&PyCArray_Type)) { + if (Py_TYPE(new) != (PyTypeObject*)metaclass) { PyErr_SetString(PyExc_AssertionError, - "MetaType appears not correctly inherited from ABC!"); - Py_DECREF(new); - return NULL; + "Metaclass not set properly!"); + goto finally; } - Py_DECREF(new); - Py_RETURN_NONE; + result = Py_NewRef(Py_None); + +finally: + Py_XDECREF(metaclass); + Py_XDECREF(class); + Py_XDECREF(new); + return result; } static PyObject * test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject *bases = PyTuple_Pack(2, PyCArray_Type, PyCFuncPtr_Type); + PyObject *metaclass_a = NULL; + PyObject *metaclass_b = NULL; + PyObject *class_a = NULL; + PyObject *class_b = NULL; + PyObject *bases = NULL; + PyObject *new = NULL; + PyObject *meta_error_string = NULL; + PyObject *exc_type = NULL; + PyObject *exc_value = NULL; + PyObject *exc_traceback = NULL; + PyObject *result = NULL; + + metaclass_a = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type); + if (metaclass_a == NULL) { + goto finally; + } + metaclass_b = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type); + if (metaclass_b == NULL) { + goto finally; + } + class_a = PyObject_CallFunction(metaclass_a, "s(){}", "TestClassA"); + if (class_a == NULL) { + goto finally; + } + + class_b = PyObject_CallFunction(metaclass_b, "s(){}", "TestClassB"); + if (class_b == NULL) { + goto finally; + } + + bases = PyTuple_Pack(2, class_a, class_b); if (bases == NULL) { - return NULL; + goto finally; } /* * The following should raise a TypeError due to a MetaClass conflict. */ - PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases); - Py_DECREF(bases); + new = PyType_FromSpecWithBases(&MinimalType_spec, bases); if (new != NULL) { - Py_DECREF(new); PyErr_SetString(PyExc_AssertionError, "MetaType conflict not recognized by PyType_FromSpecWithBases"); - return NULL; + goto finally; } - if (PyErr_ExceptionMatches(PyExc_TypeError)) { - PyObject *type, *value, *traceback, *meta_error_string; - PyErr_Fetch(&type, &value, &traceback); - Py_DECREF(type); - Py_DECREF(traceback); + // Assert that the correct exception was raised + if (PyErr_ExceptionMatches(PyExc_TypeError)) { + PyErr_Fetch(&exc_type, &exc_value, &exc_traceback); meta_error_string = PyUnicode_FromString("metaclass conflict:"); if (meta_error_string == NULL) { - Py_DECREF(value); - return NULL; + goto finally; } - int res = PyUnicode_Contains(value, meta_error_string); - Py_DECREF(value); - Py_DECREF(meta_error_string); + int res = PyUnicode_Contains(exc_value, meta_error_string); if (res < 0) { - return NULL; + goto finally; } if (res == 0) { PyErr_SetString(PyExc_AssertionError, "TypeError did not inlclude expected message."); - return NULL; + goto finally; } - Py_RETURN_NONE; + result = Py_NewRef(Py_None); } - return NULL; +finally: + Py_XDECREF(metaclass_a); + Py_XDECREF(metaclass_b); + Py_XDECREF(bases); + Py_XDECREF(new); + Py_XDECREF(meta_error_string); + Py_XDECREF(exc_type); + Py_XDECREF(exc_value); + Py_XDECREF(exc_traceback); + Py_XDECREF(class_a); + Py_XDECREF(class_b); + return result; } From 4eb003a808c321be0cdf3cacb67131dde835f904 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 27 May 2022 17:42:34 +0200 Subject: [PATCH 06/18] Make PyType_FromMetaclass refcounting easier to follow - Use separate variables for bases (strong ref) and basesnin (borrowed) - Split out bases tuple preparation - Decref held references at the end, in both success and error cases --- Objects/typeobject.c | 169 ++++++++++++++++++++++++------------------- 1 file changed, 94 insertions(+), 75 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 14ae66b2ce8ae0..95f5de610fa609 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3365,14 +3365,53 @@ static const PySlot_Offset pyslot_offsets[] = { #include "typeslots.inc" }; +/* Given a PyType_FromMetaclass `bases` argument (NULL, type, or tuple of + * types), return a tuple of types. + */ +inline static PyObject * +get_bases_tuple(PyObject *bases_in, PyType_Spec *spec) { + if (!bases_in) { + /* Default: look in the spec, fall back to (type,). */ + PyTypeObject *base = &PyBaseObject_Type; // borrowed ref + PyObject *bases = NULL; // borrowed ref + const PyType_Slot *slot; + for (slot = spec->slots; slot->slot; slot++) { + switch (slot->slot) { + case Py_tp_base: + base = slot->pfunc; + break; + case Py_tp_bases: + bases = slot->pfunc; + break; + } + } + if (!bases) { + return PyTuple_Pack(1, base); + } + if (PyTuple_Check(bases)) { + return Py_NewRef(bases); + } + PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple"); + return NULL; + } + if (PyTuple_Check(bases_in)) { + return Py_NewRef(bases_in); + } + // Not a tuple, should be a single type + return PyTuple_Pack(1, bases_in); +} + PyObject * PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, - PyType_Spec *spec, PyObject *bases) + PyType_Spec *spec, PyObject *bases_in) { PyHeapTypeObject *res = NULL; - PyObject *modname; - PyTypeObject *type, *base; + PyObject *modname = NULL; + PyTypeObject *type; + PyObject *bases = NULL; + PyObject *result = NULL; int r; + int success = 0; const PyType_Slot *slot; Py_ssize_t nmembers, weaklistoffset, dictoffset, vectorcalloffset; @@ -3410,69 +3449,39 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (spec->name == NULL) { PyErr_SetString(PyExc_SystemError, "Type spec does not define the name field."); - goto fail; + goto finally; } - /* Adjust for empty tuple bases */ + /* Get a tuple of bases. + * bases is a strong reference (unlike bases_in). + */ + bases = get_bases_tuple(bases_in, spec); if (!bases) { - base = &PyBaseObject_Type; - /* See whether Py_tp_base(s) was specified */ - for (slot = spec->slots; slot->slot; slot++) { - if (slot->slot == Py_tp_base) - base = slot->pfunc; - else if (slot->slot == Py_tp_bases) { - bases = slot->pfunc; - } - } - if (!bases) { - bases = PyTuple_Pack(1, base); - if (!bases) - goto fail; - } - else if (!PyTuple_Check(bases)) { - PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple"); - goto fail; - } - else { - Py_INCREF(bases); - } - } - else if (!PyTuple_Check(bases)) { - bases = PyTuple_Pack(1, bases); - if (!bases) - goto fail; - } - else { - Py_INCREF(bases); + goto finally; } if (!metaclass) { metaclass = &PyType_Type; } - metaclass = _PyType_CalculateMetaclass(metaclass, bases); if (metaclass == NULL) { - Py_DECREF(bases); - goto fail; + goto finally; } if (!PyType_Check(metaclass)) { - Py_DECREF(bases); PyErr_Format(PyExc_TypeError, "Metaclass '%R' is not a subclass of 'type'.", metaclass); - goto fail; + goto finally; } if (metaclass->tp_new != PyType_Type.tp_new) { - Py_DECREF(bases); PyErr_SetString(PyExc_TypeError, "Metaclasses with custom tp_new are not supported."); - goto fail; + goto finally; } res = (PyHeapTypeObject*)metaclass->tp_alloc(metaclass, nmembers); if (res == NULL) { - Py_DECREF(bases); - goto fail; + goto finally; } res_start = (char*)res; @@ -3491,7 +3500,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, res->ht_name = PyUnicode_FromString(s); if (!res->ht_name) { - goto fail; + goto finally; } res->ht_qualname = Py_NewRef(res->ht_name); @@ -3507,24 +3516,22 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, Py_ssize_t name_buf_len = strlen(spec->name) + 1; res->_ht_tpname = PyMem_Malloc(name_buf_len); if (res->_ht_tpname == NULL) { - goto fail; + goto finally; } type->tp_name = memcpy(res->_ht_tpname, spec->name, name_buf_len); res->ht_module = Py_XNewRef(module); /* Calculate best base, and check that all bases are type objects */ - base = best_base(bases); + PyTypeObject *base = best_base(bases); // borrowed ref if (base == NULL) { - Py_DECREF(bases); - goto fail; + goto finally; } if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) { PyErr_Format(PyExc_TypeError, "type '%.100s' is not an acceptable base type", base->tp_name); - Py_DECREF(bases); - goto fail; + goto finally; } /* Initialize essential fields */ @@ -3533,11 +3540,13 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, type->tp_as_sequence = &res->as_sequence; type->tp_as_mapping = &res->as_mapping; type->tp_as_buffer = &res->as_buffer; + /* Set tp_base and tp_bases */ + type->tp_base = Py_NewRef(base); type->tp_bases = bases; - Py_INCREF(base); - type->tp_base = base; + bases = NULL; // We give our reference to bases to the type + /* Copy the sizes */ type->tp_basicsize = spec->basicsize; type->tp_itemsize = spec->itemsize; @@ -3545,7 +3554,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (slot->slot < 0 || (size_t)slot->slot >= Py_ARRAY_LENGTH(pyslot_offsets)) { PyErr_SetString(PyExc_RuntimeError, "invalid slot offset"); - goto fail; + goto finally; } else if (slot->slot == Py_tp_base || slot->slot == Py_tp_bases) { /* Processed above */ @@ -3563,7 +3572,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (tp_doc == NULL) { type->tp_doc = NULL; PyErr_NoMemory(); - goto fail; + goto finally; } memcpy(tp_doc, slot->pfunc, len); type->tp_doc = tp_doc; @@ -3598,8 +3607,9 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, type->tp_vectorcall_offset = vectorcalloffset; } - if (PyType_Ready(type) < 0) - goto fail; + if (PyType_Ready(type) < 0) { + goto finally; + } if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { res->ht_cached_keys = _PyDict_NewKeysForClass(); @@ -3607,29 +3617,33 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (type->tp_doc) { PyObject *__doc__ = PyUnicode_FromString(_PyType_DocWithoutSignature(type->tp_name, type->tp_doc)); - if (!__doc__) - goto fail; + if (!__doc__) { + goto finally; + } r = PyDict_SetItem(type->tp_dict, &_Py_ID(__doc__), __doc__); Py_DECREF(__doc__); - if (r < 0) - goto fail; + if (r < 0) { + goto finally; + } } if (weaklistoffset) { type->tp_weaklistoffset = weaklistoffset; - if (PyDict_DelItemString((PyObject *)type->tp_dict, "__weaklistoffset__") < 0) - goto fail; + if (PyDict_DelItemString((PyObject *)type->tp_dict, "__weaklistoffset__") < 0) { + goto finally; + } } if (dictoffset) { type->tp_dictoffset = dictoffset; - if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0) - goto fail; + if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0) { + goto finally; + } } /* Set type.__module__ */ r = PyDict_Contains(type->tp_dict, &_Py_ID(__module__)); if (r < 0) { - goto fail; + goto finally; } if (r == 0) { s = strrchr(spec->name, '.'); @@ -3637,26 +3651,31 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, modname = PyUnicode_FromStringAndSize( spec->name, (Py_ssize_t)(s - spec->name)); if (modname == NULL) { - goto fail; + goto finally; } r = PyDict_SetItem(type->tp_dict, &_Py_ID(__module__), modname); - Py_DECREF(modname); - if (r != 0) - goto fail; - } else { + if (r != 0) { + goto finally; + } + } + else { if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1, "builtin type %.200s has no __module__ attribute", spec->name)) - goto fail; + goto finally; } } assert(_PyType_CheckConsistency(type)); - return (PyObject*)res; + success = 1; - fail: - Py_XDECREF(res); - return NULL; + finally: + if (!success) { + Py_CLEAR(res); + } + Py_XDECREF(bases); + Py_XDECREF(modname); + return (PyObject*)res; } PyObject * From 1ae7969b99c50f7b2a2a943e3740f5634d612c41 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 27 May 2022 18:24:30 +0200 Subject: [PATCH 07/18] Adjust documentation --- Doc/c-api/type.rst | 21 +++++++++++++++++-- .../2021-10-05-21-59-43.bpo-45383.TVClgf.rst | 4 +++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 99b3845237d868..20be48ffedb60f 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -196,8 +196,9 @@ The following functions and structs are used to create (:const:`Py_TPFLAGS_HEAPTYPE`). The metaclass *metaclass* is used to construct the resulting type object. - When *metaclass* is ``NULL``, the default :c:type:`PyType_Type` is used - instead. Note that metaclasses that override + When *metaclass* is ``NULL``, the metaclass is derived from *bases* + (or *Py_tp_base[s]* slots if *bases* is ``NULL``, see below). + Note that metaclasses that override :c:member:`~PyTypeObject.tp_new` are not supported. The *bases* argument can be used to specify base classes; it can either @@ -228,6 +229,11 @@ The following functions and structs are used to create The function now accepts a single class as the *bases* argument and ``NULL`` as the ``tp_doc`` slot. + .. versionchanged:: 3.12 + + The function now finds and uses a metaclass corresponding to the provided + base classes. Previously, only :class:`type` instances were returned. + .. c:function:: PyObject* PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) @@ -235,10 +241,21 @@ The following functions and structs are used to create .. versionadded:: 3.3 + .. versionchanged:: 3.12 + + The function now finds and uses a metaclass corresponding to the provided + base classes. Previously, only :class:`type` instances were returned. + .. c:function:: PyObject* PyType_FromSpec(PyType_Spec *spec) Equivalent to ``PyType_FromMetaclass(NULL, NULL, spec, NULL)``. + .. versionchanged:: 3.12 + + The function now finds and uses a metaclass corresponding to the + base classes provided in *Py_tp_base[s]* slots. + Previously, only :class:`type` instances were returned. + .. c:type:: PyType_Spec Structure defining a type's behavior. diff --git a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst index 3e9f6f484d8d89..9171e1770d016e 100644 --- a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst +++ b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst @@ -1 +1,3 @@ -The `PyType_FromSpec` API will now find the correct MetaClass from the provided bases and call the ``alloc`` function of the MetaType. An error will be raised if there is a MetaClass conflict. \ No newline at end of file +The `PyType_FromSpec` API will now find and use a metaclass +based on the provided bases. +An error will be raised if there is a metaclass conflict. From 6001a3322fc287fe4f4c1dd6a86e25c1d4cbc7d0 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 12:38:43 +0200 Subject: [PATCH 08/18] Fix type warning --- Objects/typeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 95f5de610fa609..90005210758ac1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3542,7 +3542,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, type->tp_as_buffer = &res->as_buffer; /* Set tp_base and tp_bases */ - type->tp_base = Py_NewRef(base); + type->tp_base = (PyObject *)Py_NewRef(base); type->tp_bases = bases; bases = NULL; // We give our reference to bases to the type From 1ad0ecadb217d42886e9abe4d08c25d76124d743 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 12:40:34 +0200 Subject: [PATCH 09/18] Remove unused includes from _testcapimodule.c --- Modules/_testcapimodule.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 125e7b1aa6320b..581451cf810af3 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -28,9 +28,6 @@ #include // FLT_MAX #include -#include /* required by ctypes.h */ -#include "_ctypes/ctypes.h" /* To test metaclass inheritance */ - #ifdef MS_WINDOWS # include // struct timeval #endif From ea6981952ad9d418fef890fdfc35f59b69d502b7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 12:41:28 +0200 Subject: [PATCH 10/18] Use an explicit role in the News entry --- Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst index 9171e1770d016e..ca1b7a43d29481 100644 --- a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst +++ b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst @@ -1,3 +1,3 @@ -The `PyType_FromSpec` API will now find and use a metaclass +The :c:func:`PyType_FromSpec` API will now find and use a metaclass based on the provided bases. An error will be raised if there is a metaclass conflict. From cb1527ff40107f1d3d64799008ac599ab8e5ad9a Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 13:05:07 +0200 Subject: [PATCH 11/18] Test that __subclasses__ is updated properly --- Modules/_testcapimodule.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 581451cf810af3..603e5cbdb4e5d9 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1232,7 +1232,9 @@ test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored) PyObject *metaclass = NULL; PyObject *class = NULL; PyObject *new = NULL; + PyObject *subclasses = NULL; PyObject *result = NULL; + int r; metaclass = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type); if (metaclass == NULL) { @@ -1252,12 +1254,29 @@ test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored) "Metaclass not set properly!"); goto finally; } + + /* Assert that __subclasses__ is updated */ + subclasses = PyObject_CallMethod(class, "__subclasses__", ""); + if (!subclasses) { + goto finally; + } + r = PySequence_Contains(subclasses, new); + if (r < 0) { + goto finally; + } + if (r == 0) { + PyErr_SetString(PyExc_AssertionError, + "subclasses not set properly!"); + goto finally; + } + result = Py_NewRef(Py_None); finally: Py_XDECREF(metaclass); Py_XDECREF(class); Py_XDECREF(new); + Py_XDECREF(subclasses); return result; } From f86d2b5a6aa47feb8199ab25b51f3c330594ec2d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 15:27:27 +0200 Subject: [PATCH 12/18] Document caveats, suggest that PyType_From* is not for user-provided classes --- Doc/c-api/type.rst | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 20be48ffedb60f..fece3e6e642ff5 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -193,7 +193,7 @@ The following functions and structs are used to create .. c:function:: PyObject* PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, PyType_Spec *spec, PyObject *bases) Create and return a :ref:`heap type ` from the *spec* - (:const:`Py_TPFLAGS_HEAPTYPE`). + (see :const:`Py_TPFLAGS_HEAPTYPE`). The metaclass *metaclass* is used to construct the resulting type object. When *metaclass* is ``NULL``, the metaclass is derived from *bases* @@ -216,6 +216,19 @@ The following functions and structs are used to create This function calls :c:func:`PyType_Ready` on the new type. + Note that this function does *not* fully match the behavior of + calling :py:class:`type() ` or using the :keyword:`class` statement. + With user-provided base types or metaclasses, prefer + :ref:`calling ` :py:class:`type` (or the metaclass) + over ``PyType_From*`` functions. + Specifically: + + * :py:meth:`~object.__new__` is not called on the new class + (and it must be set to ``type.__new__``). + * :py:meth:`~object.__init__` is not called on the new class. + * :py:meth:`~object.__init_subclass__` is not called on any bases. + * :py:meth:`~object.__set_name__` is not called on new descriptors. + .. versionadded:: 3.12 .. c:function:: PyObject* PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) From 93c65e81e94d33eee6d1c087c933ac46486dd77c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 16:04:26 +0200 Subject: [PATCH 13/18] Remove unused variable --- Objects/typeobject.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 90005210758ac1..a950db492617ef 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3409,7 +3409,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, PyObject *modname = NULL; PyTypeObject *type; PyObject *bases = NULL; - PyObject *result = NULL; int r; int success = 0; From d881750fece555526a24caba53dc1d8a838ee4ec Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 31 May 2022 16:04:41 +0200 Subject: [PATCH 14/18] Move bases calculation before the allocation --- Objects/typeobject.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a950db492617ef..752b669ae460ed 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3459,6 +3459,8 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, goto finally; } + /* Calculate the metaclass */ + if (!metaclass) { metaclass = &PyType_Type; } @@ -3478,6 +3480,20 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, goto finally; } + /* Calculate best base, and check that all bases are type objects */ + PyTypeObject *base = best_base(bases); // borrowed ref + if (base == NULL) { + goto finally; + } + if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) { + PyErr_Format(PyExc_TypeError, + "type '%.100s' is not an acceptable base type", + base->tp_name); + goto finally; + } + + /* Allocate the new type */ + res = (PyHeapTypeObject*)metaclass->tp_alloc(metaclass, nmembers); if (res == NULL) { goto finally; @@ -3521,18 +3537,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, res->ht_module = Py_XNewRef(module); - /* Calculate best base, and check that all bases are type objects */ - PyTypeObject *base = best_base(bases); // borrowed ref - if (base == NULL) { - goto finally; - } - if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) { - PyErr_Format(PyExc_TypeError, - "type '%.100s' is not an acceptable base type", - base->tp_name); - goto finally; - } - /* Initialize essential fields */ type->tp_as_async = &res->as_async; type->tp_as_number = &res->as_number; From a7af972e72ad6ee70fb241f66b511d7feea41b4e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 1 Jun 2022 16:27:58 +0200 Subject: [PATCH 15/18] Fix compiler warning for Py_NewRef result --- Objects/typeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 752b669ae460ed..19088fa5cc6097 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3545,7 +3545,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, type->tp_as_buffer = &res->as_buffer; /* Set tp_base and tp_bases */ - type->tp_base = (PyObject *)Py_NewRef(base); + type->tp_base = (PyTypeObject *)Py_NewRef(base); type->tp_bases = bases; bases = NULL; // We give our reference to bases to the type From 2f47be342c056c4008ea111a11cf8dadb5f03d0e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Jun 2022 16:09:29 +0200 Subject: [PATCH 16/18] Use PyErr_Occurred rather than a variable --- Objects/typeobject.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 19088fa5cc6097..83ac0247b261c0 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3410,7 +3410,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, PyTypeObject *type; PyObject *bases = NULL; int r; - int success = 0; const PyType_Slot *slot; Py_ssize_t nmembers, weaklistoffset, dictoffset, vectorcalloffset; @@ -3670,10 +3669,9 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, } assert(_PyType_CheckConsistency(type)); - success = 1; finally: - if (!success) { + if (PyErr_Occurred()) { Py_CLEAR(res); } Py_XDECREF(bases); From ef6bccecd78f76f5015e4667f0dcde60f53dc26c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Jun 2022 16:11:26 +0200 Subject: [PATCH 17/18] Use PEP 7 style --- Objects/typeobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 83ac0247b261c0..321ae9b9ed2595 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3369,7 +3369,8 @@ static const PySlot_Offset pyslot_offsets[] = { * types), return a tuple of types. */ inline static PyObject * -get_bases_tuple(PyObject *bases_in, PyType_Spec *spec) { +get_bases_tuple(PyObject *bases_in, PyType_Spec *spec) +{ if (!bases_in) { /* Default: look in the spec, fall back to (type,). */ PyTypeObject *base = &PyBaseObject_Type; // borrowed ref From 6b31880cf77b079386844e9d3a43cdcc712d3264 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Jun 2022 16:13:25 +0200 Subject: [PATCH 18/18] Replace duplicate Py_TPFLAGS_BASETYPE check with an assert --- Objects/typeobject.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 321ae9b9ed2595..696a19b96bea89 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3485,12 +3485,9 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (base == NULL) { goto finally; } - if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) { - PyErr_Format(PyExc_TypeError, - "type '%.100s' is not an acceptable base type", - base->tp_name); - goto finally; - } + // best_base should check Py_TPFLAGS_BASETYPE & raise a proper exception, + // here we just check its work + assert(_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)); /* Allocate the new type */