Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-117482: Simplify the Fix For Builtin Types Slot Wrappers #122865

1 change: 0 additions & 1 deletion Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ struct _types_runtime_state {
struct {
struct {
PyTypeObject *type;
PyTypeObject def;
int64_t interp_count;
} types[_Py_MAX_MANAGED_STATIC_TYPES];
} managed_static;
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_doctest/test_doctest.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def non_Python_modules(): r"""

>>> import builtins
>>> tests = doctest.DocTestFinder().find(builtins)
>>> 830 < len(tests) < 860 # approximate number of objects with docstrings
>>> 750 < len(tests) < 800 # approximate number of objects with docstrings
True
>>> real_tests = [t for t in tests if len(t.examples) > 0]
>>> len(real_tests) # objects that actually have doctests
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,6 @@ def test_datetime_reset_strptime(self):
out, err = self.run_embedded_interpreter("test_repeated_init_exec", code)
self.assertEqual(out, '20000101\n' * INIT_LOOPS)

@unittest.skip('inheritance across re-init is currently broken; see gh-117482')
def test_static_types_inherited_slots(self):
script = textwrap.dedent("""
import test.support
Expand Down
10 changes: 0 additions & 10 deletions Lib/test/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2410,9 +2410,6 @@ def test_static_types_inherited_slots(self):
def collate_results(raw):
results = {}
for cls, attr, wrapper in raw:
# XXX This should not be necessary.
if cls == repr(bool) and attr in self.NUMERIC_METHODS:
continue
key = cls, attr
assert key not in results, (results, key, wrapper)
results[key] = wrapper
Expand All @@ -2433,14 +2430,7 @@ def collate_results(raw):
cls, attr = key
with self.subTest(cls=cls, slotattr=attr):
actual = interp_results.pop(key)
# XXX This should not be necessary.
if cls == "<class 'collections.OrderedDict'>" and attr == '__len__':
continue
self.assertEqual(actual, expected)
# XXX This should not be necessary.
interp_results = {k: v for k, v in interp_results.items() if k[1] != '__hash__'}
# XXX This should not be necessary.
interp_results.pop(("<class 'collections.OrderedDict'>", '__getitem__'), None)
self.maxDiff = None
self.assertEqual(interp_results, {})

Expand Down
66 changes: 37 additions & 29 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -314,15 +314,6 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self,
}
}

static PyTypeObject *
managed_static_type_get_def(PyTypeObject *self, int isbuiltin)
{
size_t index = managed_static_type_index_get(self);
size_t full_index = isbuiltin
? index
: index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES;
return &_PyRuntime.types.managed_static.types[full_index].def;
}


PyObject *
Expand Down Expand Up @@ -5882,6 +5873,7 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,

_PyStaticType_ClearWeakRefs(interp, type);
managed_static_type_state_clear(interp, type, isbuiltin, final);
/* We leave _Py_TPFLAGS_STATIC_BUILTIN set on tp_flags. */
}

void
Expand Down Expand Up @@ -7894,7 +7886,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
return 0;
}

static int add_operators(PyTypeObject *, PyTypeObject *);
static int add_operators(PyTypeObject *type);
static int add_tp_new_wrapper(PyTypeObject *type);

#define COLLECTION_FLAGS (Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_MAPPING)
Expand Down Expand Up @@ -8059,10 +8051,10 @@ type_dict_set_doc(PyTypeObject *type)


static int
type_ready_fill_dict(PyTypeObject *type, PyTypeObject *def)
type_ready_fill_dict(PyTypeObject *type)
{
/* Add type-specific descriptors to tp_dict */
if (add_operators(type, def) < 0) {
if (add_operators(type) < 0) {
return -1;
}
if (type_add_methods(type) < 0) {
Expand Down Expand Up @@ -8381,7 +8373,7 @@ type_ready_post_checks(PyTypeObject *type)


static int
type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
type_ready(PyTypeObject *type, int initial)
{
ASSERT_TYPE_LOCK_HELD();

Expand Down Expand Up @@ -8420,7 +8412,7 @@ type_ready(PyTypeObject *type, PyTypeObject *def, int initial)
if (type_ready_set_new(type, initial) < 0) {
goto error;
}
if (type_ready_fill_dict(type, def) < 0) {
if (type_ready_fill_dict(type) < 0) {
goto error;
}
if (initial) {
Expand Down Expand Up @@ -8477,7 +8469,7 @@ PyType_Ready(PyTypeObject *type)
int res;
BEGIN_TYPE_LOCK();
if (!(type->tp_flags & Py_TPFLAGS_READY)) {
res = type_ready(type, NULL, 1);
res = type_ready(type, 1);
} else {
res = 0;
assert(_PyType_CheckConsistency(type));
Expand Down Expand Up @@ -8513,20 +8505,14 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,

managed_static_type_state_init(interp, self, isbuiltin, initial);

PyTypeObject *def = managed_static_type_get_def(self, isbuiltin);
if (initial) {
memcpy(def, self, sizeof(PyTypeObject));
}

int res;
BEGIN_TYPE_LOCK();
res = type_ready(self, def, initial);
res = type_ready(self, initial);
END_TYPE_LOCK();
if (res < 0) {
_PyStaticType_ClearWeakRefs(interp, self);
managed_static_type_state_clear(interp, self, isbuiltin, initial);
}

return res;
}

Expand Down Expand Up @@ -11101,6 +11087,26 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
return 0;
}

static int
slot_inherited(PyTypeObject *type, pytype_slotdef *slotdef, void **slot)
{
void **slot_base = slotptr(type->tp_base, slotdef->offset);
if (slot_base == NULL || *slot != *slot_base) {
return 0;
}

/* Some slots are inherited in pairs. */
if (slot == (void *)&type->tp_hash) {
return (type->tp_richcompare == type->tp_base->tp_richcompare);
}
else if (slot == (void *)&type->tp_richcompare) {
return (type->tp_hash == type->tp_base->tp_hash);
}

/* It must be inherited (see type_ready_inherit()). */
return 1;
}

/* This function is called by PyType_Ready() to populate the type's
dictionary with method descriptors for function slots. For each
function slot (like tp_repr) that's defined in the type, one or more
Expand Down Expand Up @@ -11132,24 +11138,26 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name,
infinite recursion here.) */

static int
add_operators(PyTypeObject *type, PyTypeObject *def)
add_operators(PyTypeObject *type)
{
PyObject *dict = lookup_tp_dict(type);
pytype_slotdef *p;
PyObject *descr;
void **ptr;

assert(def == NULL || (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
if (def == NULL) {
def = type;
}

for (p = slotdefs; p->name; p++) {
if (p->wrapper == NULL)
continue;
ptr = slotptr(def, p->offset);
ptr = slotptr(type, p->offset);
if (!ptr || !*ptr)
continue;
/* Also ignore when the type slot has been inherited. */
if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN
&& type->tp_base != NULL
&& slot_inherited(type, p, ptr))
{
continue;
}
int r = PyDict_Contains(dict, p->name_strobj);
if (r > 0)
continue;
Expand Down
Loading