Index: Lib/test/test_descr.py =================================================================== --- Lib/test/test_descr.py (revision 55256) +++ Lib/test/test_descr.py (working copy) @@ -4354,6 +4354,45 @@ c[1:2] = 3 vereq(c.value, 3) +def test_slot_mixing(): + """ + Bug #1694663: there used to be a problem when a class had slot + wrappers for special methods, and those wrappers didn't come from + the base class. The problem also showed when a slot wrapper had + an unexpected conversion function. + """ + class B(object): + __new__ = int.__new__ + class C(int): + __new__ = object.__new__ + for cls in B, C: + try: + cls() + except TypeError: + pass + else: + msg = "%r() should raise a TypeError because its __new__ is broken" + raise TestFailed(msg % cls.__name__) + class NotReallyInt(int): + __mul__ = float.__add__ + __mod__ = int.__neg__ + __pow__ = int.__add__ + try: + NotReallyInt(3) * 3 + except TypeError: + pass + else: + raise TestFailed("NotReallyInt(3) * 3 should raise a TypeError " + "because NotReallyInt.__mul__ == float.__add__") + try: + NotReallyInt(3) % 3 + except TypeError: + pass + else: + raise TestFailed("NotReallyInt(3) % 3 should raise a TypeError " + "because NotReallyInt.__mod__ == int.__neg__") + vereq(NotReallyInt(3) ** 3, 6) + def test_main(): weakref_segfault() # Must be first, somehow wrapper_segfault() @@ -4451,6 +4490,7 @@ methodwrapper() notimplemented() test_assign_slice() + test_slot_mixing() if verbose: print "All OK" Index: Objects/typeobject.c =================================================================== --- Objects/typeobject.c (revision 55256) +++ Objects/typeobject.c (working copy) @@ -5490,117 +5490,106 @@ return (void **)ptr; } -/* Length of array of slotdef pointers used to store slots with the - same __name__. There should be at most MAX_EQUIV-1 slotdef entries with - the same __name__, for any __name__. Since that's a static property, it is - appropriate to declare fixed-size arrays for this. */ -#define MAX_EQUIV 10 - -/* Return a slot pointer for a given name, but ONLY if the attribute has - exactly one slot function. The name must be an interned string. */ -static void ** -resolve_slotdups(PyTypeObject *type, PyObject *name) +/* Look for a base from which 'type' has inherited slot method 'name'. */ +static PyTypeObject * +lookup_slot_provider(PyTypeObject *type, PyObject *name) { - /* XXX Maybe this could be optimized more -- but is it worth it? */ + Py_ssize_t i, n; + PyObject *mro, *base, *dict; + PyTypeObject *t; - /* pname and ptrs act as a little cache */ - static PyObject *pname; - static slotdef *ptrs[MAX_EQUIV]; - slotdef *p, **pp; - void **res, **ptr; + mro = type->tp_mro; + n = PyTuple_GET_SIZE(mro); - if (pname != name) { - /* Collect all slotdefs that match name into ptrs. */ - pname = name; - pp = ptrs; - for (p = slotdefs; p->name_strobj; p++) { - if (p->name_strobj == name) - *pp++ = p; + /* type might have a custom MRO, so we can't just + start at index 1 to skip the type itself. */ + for (i = 0; i < n; i++) { + base = PyTuple_GET_ITEM(mro, i); + if (PyClass_Check(base)) { + /* Old style classes don't provide slots. */ + dict = ((PyClassObject *)base)->cl_dict; + if (PyDict_GetItem(dict, name) != NULL) + return NULL; } - *pp = NULL; + else { + t = (PyTypeObject *)base; + if (t == type) + continue; + dict = t->tp_dict; + if (PyDict_GetItem(dict, name) != NULL) + return t; + } } - - /* Look in all matching slots of the type; if exactly one of these has - a filled-in slot, return its value. Otherwise return NULL. */ - res = NULL; - for (pp = ptrs; *pp; pp++) { - ptr = slotptr(type, (*pp)->offset); - if (ptr == NULL || *ptr == NULL) - continue; - if (res != NULL) - return NULL; - res = ptr; - } - return res; + return NULL; } /* Common code for update_slots_callback() and fixup_slot_dispatchers(). This does some incredibly complex thinking and then sticks something into the - slot. (It sees if the adjacent slotdefs for the same slot have conflicting - interests, and then stores a generic wrapper or a specific function into - the slot.) Return a pointer to the next slotdef with a different offset, + slot. + + This function has three responsibilities: + - Clear type's slot when type doesn't have any of the __special__ + methods coresponding to 'slotdef'. + - Fill type's slot with a wrapper when such a method is found. + - If possible, fill type's slot with an optimised function instead + of the generic wrapper. + + The first two goals are easy to achieve; we simply look for all + __special__ aliases for a given slot offset. If there is at + least one such method, we put a generic wrapper into the type + structure, otherwise we set the slot to NULL. + + The third goal is more delicate. If a type inherits all of the + __special__ methods for a given slot offset from the same base, + then we can use the -- possibly optimised -- slot function from + that base instead of a generic wrapper. + Note that this might set the slot to NULL when different slots + are known by the same name (eg. if a base has the sq_item slot, + but is missing the mp_subscript slot, then its subclasses that + don't overwrite __getitem__ also won't have a mp_subscript slot). + This is intentional, because it speeds up the operations provided + by those slots for subclasses of builtin types. + + Return a pointer to the next slotdef with a different offset, because that's convenient for fixup_slot_dispatchers(). */ static slotdef * update_one_slot(PyTypeObject *type, slotdef *p) { PyObject *descr; - PyWrapperDescrObject *d; void *generic = NULL, *specific = NULL; int use_generic = 0; int offset = p->offset; void **ptr = slotptr(type, offset); + void **baseptr; + PyTypeObject *base, *oldbase = NULL; - if (ptr == NULL) { - do { - ++p; - } while (p->offset == offset); - return p; - } + /* Heap types always have valid indirection pointers. */ + assert(ptr != NULL); + do { descr = _PyType_Lookup(type, p->name_strobj); if (descr == NULL) continue; - if (descr->ob_type == &PyWrapperDescr_Type) { - void **tptr = resolve_slotdups(type, p->name_strobj); - if (tptr == NULL || tptr == ptr) - generic = p->function; - d = (PyWrapperDescrObject *)descr; - if (d->d_base->wrapper == p->wrapper && - PyType_IsSubtype(type, d->d_type)) - { - if (specific == NULL || - specific == d->d_wrapped) - specific = d->d_wrapped; - else - use_generic = 1; - } - } - else if (descr->ob_type == &PyCFunction_Type && - PyCFunction_GET_FUNCTION(descr) == - (PyCFunction)tp_new_wrapper && - strcmp(p->name, "__new__") == 0) - { - /* The __new__ wrapper is not a wrapper descriptor, - so must be special-cased differently. - If we don't do this, creating an instance will - always use slot_tp_new which will look up - __new__ in the MRO which will call tp_new_wrapper - which will look through the base classes looking - for a static base and call its tp_new (usually - PyType_GenericNew), after performing various - sanity checks and constructing a new argument - list. Cut all that nonsense short -- this speeds - up instance creation tremendously. */ - specific = (void *)type->tp_new; - /* XXX I'm not 100% sure that there isn't a hole - in this reasoning that requires additional - sanity checks. I'll buy the first person to - point out a bug in this reasoning a beer. */ - } - else { + base = lookup_slot_provider(type, p->name_strobj); + + /* We use the optimised slot only if: + - the wrapper is inherited from a new style base, + - the base is compiled against the same version of Python, + - all wrappers for a slot are inherited from one base, + - the type and the base have identical wrappers. */ + if (base == NULL || + !PyType_HasFeature(base, Py_TPFLAGS_DEFAULT) || + (oldbase != NULL && base != oldbase) || + PyDict_GetItem(base->tp_dict, p->name_strobj) != descr) { use_generic = 1; generic = p->function; } + else { + baseptr = slotptr(base, offset); + if (baseptr != NULL) + specific = *baseptr; + } + oldbase = base; } while ((++p)->offset == offset); if (specific && !use_generic) *ptr = specific; @@ -5656,6 +5645,12 @@ initialized = 1; } +/* Length of array of slotdef pointers used to store slots with the + same __name__. There should be at most MAX_EQUIV-1 slotdef entries with + the same __name__, for any __name__. Since that's a static property, it is + appropriate to declare fixed-size arrays for this. */ +#define MAX_EQUIV 10 + /* Update the slots after assignment to a class (type) attribute. */ static int update_slot(PyTypeObject *type, PyObject *name)