From b5de219d1ed6d5a739843bd155f7fd9bd85c26ca Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Thu, 27 Apr 2017 08:45:43 -0700 Subject: [PATCH 1/2] bpo-30140: Fix binary operator dispatch for subclasses --- Lib/test/test_binop.py | 34 +++++++++++++++++++++++++++++++++- Lib/test/test_descr.py | 12 ------------ Objects/typeobject.c | 38 +------------------------------------- 3 files changed, 34 insertions(+), 50 deletions(-) diff --git a/Lib/test/test_binop.py b/Lib/test/test_binop.py index 4f5b8a8b93ed47..842aa7283b1f6e 100644 --- a/Lib/test/test_binop.py +++ b/Lib/test/test_binop.py @@ -1,7 +1,7 @@ """Tests for binary operators on subtypes of built-in types.""" import unittest -from operator import eq, le, ne +from operator import eq, le, ne, add from abc import ABCMeta def gcd(a, b): @@ -387,6 +387,38 @@ def test_comparison_orders(self): self.assertEqual(op_sequence(eq, B, V), ['B.__eq__', 'V.__eq__']) self.assertEqual(op_sequence(le, B, V), ['B.__le__', 'V.__ge__']) + def test_arithmetic_orders(self): + + def logged_op(name): + def op(self, other): + type_name = type(self).__name__ + self.log_operation(f'{type_name}.__{name}__') + return NotImplemented + return op + + class A(OperationLogger): + __add__ = logged_op('add') + __radd__ = logged_op('radd') + + class B(OperationLogger): + __add__ = logged_op('add') + __radd__ = logged_op('radd') + + class C(A): + pass + + class D(OperationLogger): + pass + + self.assertEqual(op_sequence(add, A, A), ['A.__add__']) + self.assertEqual(op_sequence(add, A, D), ['A.__add__']) + self.assertEqual(op_sequence(add, D, A), ['A.__radd__']) + self.assertEqual(op_sequence(add, A, B), ['A.__add__', 'B.__radd__']) + self.assertEqual(op_sequence(add, B, A), ['B.__add__', 'A.__radd__']) + self.assertEqual(op_sequence(add, A, C), ['C.__radd__', 'A.__add__']) + self.assertEqual(op_sequence(add, C, A), ['C.__add__', 'A.__radd__']) + + class SupEq(object): """Class that can test equality""" def __eq__(self, other): diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index ced25f3fc44244..afa37f08b5c7f0 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4111,18 +4111,6 @@ def __rfloordiv__(self, other): self.assertEqual(D() // C(), "D.__floordiv__") self.assertEqual(C() // D(), "D.__rfloordiv__") - # Case 4: this didn't work right in 2.2.2 and 2.3a1 - - class E(C): - pass - - self.assertEqual(E.__rfloordiv__, C.__rfloordiv__) - - self.assertEqual(E() // 1, "C.__floordiv__") - self.assertEqual(1 // E(), "C.__rfloordiv__") - self.assertEqual(E() // C(), "C.__floordiv__") - self.assertEqual(C() // E(), "C.__floordiv__") # This one would fail - @support.impl_detail("testing an internal kind of method object") def test_meth_class_get(self): # Testing __get__ method of METH_CLASS C methods... diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9eb725f062ba56..fdf0584323cbfd 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5841,41 +5841,6 @@ FUNCNAME(PyObject *self, ARG1TYPE arg1) \ return call_method(self, &id, stack, 1); \ } -/* Boolean helper for SLOT1BINFULL(). - right.__class__ is a nontrivial subclass of left.__class__. */ -static int -method_is_overloaded(PyObject *left, PyObject *right, struct _Py_Identifier *name) -{ - PyObject *a, *b; - int ok; - - b = _PyObject_GetAttrId((PyObject *)(Py_TYPE(right)), name); - if (b == NULL) { - PyErr_Clear(); - /* If right doesn't have it, it's not overloaded */ - return 0; - } - - a = _PyObject_GetAttrId((PyObject *)(Py_TYPE(left)), name); - if (a == NULL) { - PyErr_Clear(); - Py_DECREF(b); - /* If right has it but left doesn't, it's overloaded */ - return 1; - } - - ok = PyObject_RichCompareBool(a, b, Py_NE); - Py_DECREF(a); - Py_DECREF(b); - if (ok < 0) { - PyErr_Clear(); - return 0; - } - - return ok; -} - - #define SLOT1BINFULL(FUNCNAME, TESTFUNC, SLOTNAME, OPSTR, ROPSTR) \ static PyObject * \ FUNCNAME(PyObject *self, PyObject *other) \ @@ -5890,8 +5855,7 @@ FUNCNAME(PyObject *self, PyObject *other) \ Py_TYPE(self)->tp_as_number->SLOTNAME == TESTFUNC) { \ PyObject *r; \ if (do_other && \ - PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)) && \ - method_is_overloaded(self, other, &rop_id)) { \ + PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self))) { \ stack[0] = self; \ r = call_maybe(other, &rop_id, stack, 1); \ if (r != Py_NotImplemented) \ From 52d67e8489b6bd2cc8466372528cfef6f04d5f8a Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Tue, 5 Sep 2017 14:55:07 -0700 Subject: [PATCH 2/2] bpo-30140: Add blurb --- .../Core and Builtins/2017-09-05-14-54-30.bpo-30140.fnPWpS.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-05-14-54-30.bpo-30140.fnPWpS.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-05-14-54-30.bpo-30140.fnPWpS.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-05-14-54-30.bpo-30140.fnPWpS.rst new file mode 100644 index 00000000000000..9f570815e890bb --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-05-14-54-30.bpo-30140.fnPWpS.rst @@ -0,0 +1,2 @@ +Fix binary operator dispatch to try subclasses implementations first always, +even if the subclass does not directly override the relevant special method.