diff --git a/Lib/test/test_binop.py b/Lib/test/test_binop.py index 5d77f234fe6..c7987b2013a 100644 --- a/Lib/test/test_binop.py +++ b/Lib/test/test_binop.py @@ -2,6 +2,7 @@ import unittest from test import support +from operator import eq, ne, lt, gt, le, ge def gcd(a, b): """Greatest common divisor using Euclid's algorithm.""" @@ -305,9 +306,78 @@ class RatTestCase(unittest.TestCase): # XXX Ran out of steam; TO DO: divmod, div, future division -def test_main(): - support.run_unittest(RatTestCase) +class OperationLogger: + """Base class for classes with operation logging.""" + def __init__(self, logger): + self.logger = logger + def log_operation(self, *args): + self.logger(*args) + +def op_sequence(op, *classes): + """Return the sequence of operations that results from applying + the operation `op` to instances of the given classes.""" + log = [] + instances = [] + for c in classes: + instances.append(c(log.append)) + + try: + op(*instances) + except TypeError: + pass + return log + +class A(OperationLogger): + def __eq__(self, other): + self.log_operation('A.__eq__') + return NotImplemented + def __le__(self, other): + self.log_operation('A.__le__') + return NotImplemented + def __ge__(self, other): + self.log_operation('A.__ge__') + return NotImplemented + +class B(OperationLogger): + def __eq__(self, other): + self.log_operation('B.__eq__') + return NotImplemented + def __le__(self, other): + self.log_operation('B.__le__') + return NotImplemented + def __ge__(self, other): + self.log_operation('B.__ge__') + return NotImplemented + +class C(B): + def __eq__(self, other): + self.log_operation('C.__eq__') + return NotImplemented + def __le__(self, other): + self.log_operation('C.__le__') + return NotImplemented + def __ge__(self, other): + self.log_operation('C.__ge__') + return NotImplemented + +class OperationOrderTests(unittest.TestCase): + def test_comparison_orders(self): + self.assertEqual(op_sequence(eq, A, A), ['A.__eq__', 'A.__eq__']) + self.assertEqual(op_sequence(eq, A, B), ['A.__eq__', 'B.__eq__']) + self.assertEqual(op_sequence(eq, B, A), ['B.__eq__', 'A.__eq__']) + # C is a subclass of B, so C.__eq__ is called first + self.assertEqual(op_sequence(eq, B, C), ['C.__eq__', 'B.__eq__']) + self.assertEqual(op_sequence(eq, C, B), ['C.__eq__', 'B.__eq__']) + + self.assertEqual(op_sequence(le, A, A), ['A.__le__', 'A.__ge__']) + self.assertEqual(op_sequence(le, A, B), ['A.__le__', 'B.__ge__']) + self.assertEqual(op_sequence(le, B, A), ['B.__le__', 'A.__ge__']) + self.assertEqual(op_sequence(le, B, C), ['C.__ge__', 'B.__le__']) + self.assertEqual(op_sequence(le, C, B), ['C.__le__', 'B.__ge__']) + +def test_main(): + support.run_unittest(RatTestCase, OperationOrderTests) if __name__ == "__main__": test_main() diff --git a/Misc/NEWS b/Misc/NEWS index 1577baf80b6..9480d9f61e6 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,9 @@ What's New in Python 3.2 Alpha 1? Core and Builtins ----------------- +- Issue #6970: Remove redundant calls when comparing objects that don't + implement the relevant rich comparison methods. + - Issue #7298: fixes for range and reversed(range(...)). Iteration over range(a, b, c) incorrectly gave an empty iterator when a, b and c fit in C long but the length of the range did not. Also fix diff --git a/Objects/object.c b/Objects/object.c index 90cdc74195a..002acd04be6 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -544,10 +544,12 @@ do_richcompare(PyObject *v, PyObject *w, int op) { richcmpfunc f; PyObject *res; + int checked_reverse_op = 0; if (v->ob_type != w->ob_type && PyType_IsSubtype(w->ob_type, v->ob_type) && (f = w->ob_type->tp_richcompare) != NULL) { + checked_reverse_op = 1; res = (*f)(w, v, _Py_SwappedOp[op]); if (res != Py_NotImplemented) return res; @@ -559,7 +561,7 @@ do_richcompare(PyObject *v, PyObject *w, int op) return res; Py_DECREF(res); } - if ((f = w->ob_type->tp_richcompare) != NULL) { + if (!checked_reverse_op && (f = w->ob_type->tp_richcompare) != NULL) { res = (*f)(w, v, _Py_SwappedOp[op]); if (res != Py_NotImplemented) return res; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 24866ff465f..be4b6f861ef 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5068,7 +5068,7 @@ static char *name_op[] = { }; static PyObject * -half_richcompare(PyObject *self, PyObject *other, int op) +slot_tp_richcompare(PyObject *self, PyObject *other, int op) { PyObject *func, *args, *res; static PyObject *op_str[6]; @@ -5090,28 +5090,6 @@ half_richcompare(PyObject *self, PyObject *other, int op) return res; } -static PyObject * -slot_tp_richcompare(PyObject *self, PyObject *other, int op) -{ - PyObject *res; - - if (Py_TYPE(self)->tp_richcompare == slot_tp_richcompare) { - res = half_richcompare(self, other, op); - if (res != Py_NotImplemented) - return res; - Py_DECREF(res); - } - if (Py_TYPE(other)->tp_richcompare == slot_tp_richcompare) { - res = half_richcompare(other, self, _Py_SwappedOp[op]); - if (res != Py_NotImplemented) { - return res; - } - Py_DECREF(res); - } - Py_INCREF(Py_NotImplemented); - return Py_NotImplemented; -} - static PyObject * slot_tp_iter(PyObject *self) {