bpo-1617161: Make the hash and equality of methods not depending on the value of self. (GH-7848)

* The hash of BuiltinMethodType instances no longer depends on the hash
  of __self__. It depends now on the hash of id(__self__).
* The hash and equality of ModuleType and MethodWrapperType instances no
  longer depend on the hash and equality of __self__. They depend now on
  the hash and equality of id(__self__).
* MethodWrapperType instances no longer support ordering.

6 files changed, 112 insertions, 73 deletions

diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py
index 841cac9..998452a 100644
--- a/Lib/test/test_class.py
+++ b/Lib/test/test_class.py
@@ -534,6 +534,16 @@ class ClassTests(unittest.TestCase):
         else:
             self.fail("attribute error for I.__init__ got masked")
 
+    def assertNotOrderable(self, a, b):
+        with self.assertRaises(TypeError):
+            a < b
+        with self.assertRaises(TypeError):
+            a > b
+        with self.assertRaises(TypeError):
+            a <= b
+        with self.assertRaises(TypeError):
+            a >= b
+
     def testHashComparisonOfMethods(self):
         # Test comparison and hash of methods
         class A:
@@ -544,24 +554,30 @@ class ClassTests(unittest.TestCase):
             def g(self):
                 pass
             def __eq__(self, other):
-                return self.x == other.x
+                return True
             def __hash__(self):
-                return self.x
+                raise TypeError
         class B(A):
             pass
 
         a1 = A(1)
-        a2 = A(2)
-        self.assertEqual(a1.f, a1.f)
-        self.assertNotEqual(a1.f, a2.f)
-        self.assertNotEqual(a1.f, a1.g)
-        self.assertEqual(a1.f, A(1).f)
+        a2 = A(1)
+        self.assertTrue(a1.f == a1.f)
+        self.assertFalse(a1.f != a1.f)
+        self.assertFalse(a1.f == a2.f)
+        self.assertTrue(a1.f != a2.f)
+        self.assertFalse(a1.f == a1.g)
+        self.assertTrue(a1.f != a1.g)
+        self.assertNotOrderable(a1.f, a1.f)
         self.assertEqual(hash(a1.f), hash(a1.f))
-        self.assertEqual(hash(a1.f), hash(A(1).f))
 
-        self.assertNotEqual(A.f, a1.f)
-        self.assertNotEqual(A.f, A.g)
-        self.assertEqual(B.f, A.f)
+        self.assertFalse(A.f == a1.f)
+        self.assertTrue(A.f != a1.f)
+        self.assertFalse(A.f == A.g)
+        self.assertTrue(A.f != A.g)
+        self.assertTrue(B.f == A.f)
+        self.assertFalse(B.f != A.f)
+        self.assertNotOrderable(A.f, A.f)
         self.assertEqual(hash(B.f), hash(A.f))
 
         # the following triggers a SystemError in 2.4
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index 0e7728e..b96d35c 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -4350,38 +4350,71 @@ order (MRO) for bases """
         else:
             self.fail("did not test __init__() for None return")
 
+    def assertNotOrderable(self, a, b):
+        with self.assertRaises(TypeError):
+            a < b
+        with self.assertRaises(TypeError):
+            a > b
+        with self.assertRaises(TypeError):
+            a <= b
+        with self.assertRaises(TypeError):
+            a >= b
+
     def test_method_wrapper(self):
         # Testing method-wrapper objects...
         # <type 'method-wrapper'> did not support any reflection before 2.5
-
-        # XXX should methods really support __eq__?
-
         l = []
-        self.assertEqual(l.__add__, l.__add__)
-        self.assertEqual(l.__add__, [].__add__)
-        self.assertNotEqual(l.__add__, [5].__add__)
-        self.assertNotEqual(l.__add__, l.__mul__)
+        self.assertTrue(l.__add__ == l.__add__)
+        self.assertFalse(l.__add__ != l.__add__)
+        self.assertFalse(l.__add__ == [].__add__)
+        self.assertTrue(l.__add__ != [].__add__)
+        self.assertFalse(l.__add__ == l.__mul__)
+        self.assertTrue(l.__add__ != l.__mul__)
+        self.assertNotOrderable(l.__add__, l.__add__)
         self.assertEqual(l.__add__.__name__, '__add__')
-        if hasattr(l.__add__, '__self__'):
-            # CPython
-            self.assertIs(l.__add__.__self__, l)
-            self.assertIs(l.__add__.__objclass__, list)
-        else:
-            # Python implementations where [].__add__ is a normal bound method
-            self.assertIs(l.__add__.im_self, l)
-            self.assertIs(l.__add__.im_class, list)
+        self.assertIs(l.__add__.__self__, l)
+        self.assertIs(l.__add__.__objclass__, list)
         self.assertEqual(l.__add__.__doc__, list.__add__.__doc__)
-        try:
-            hash(l.__add__)
-        except TypeError:
-            pass
-        else:
-            self.fail("no TypeError from hash([].__add__)")
+        # hash([].__add__) should not be based on hash([])
+        hash(l.__add__)
 
-        t = ()
-        t += (7,)
-        self.assertEqual(t.__add__, (7,).__add__)
-        self.assertEqual(hash(t.__add__), hash((7,).__add__))
+    def test_builtin_function_or_method(self):
+        # Not really belonging to test_descr, but introspection and
+        # comparison on <type 'builtin_function_or_method'> seems not
+        # to be tested elsewhere
+        l = []
+        self.assertTrue(l.append == l.append)
+        self.assertFalse(l.append != l.append)
+        self.assertFalse(l.append == [].append)
+        self.assertTrue(l.append != [].append)
+        self.assertFalse(l.append == l.pop)
+        self.assertTrue(l.append != l.pop)
+        self.assertNotOrderable(l.append, l.append)
+        self.assertEqual(l.append.__name__, 'append')
+        self.assertIs(l.append.__self__, l)
+        # self.assertIs(l.append.__objclass__, list) --- could be added?
+        self.assertEqual(l.append.__doc__, list.append.__doc__)
+        # hash([].append) should not be based on hash([])
+        hash(l.append)
+
+    def test_special_unbound_method_types(self):
+        # Testing objects of <type 'wrapper_descriptor'>...
+        self.assertTrue(list.__add__ == list.__add__)
+        self.assertFalse(list.__add__ != list.__add__)
+        self.assertFalse(list.__add__ == list.__mul__)
+        self.assertTrue(list.__add__ != list.__mul__)
+        self.assertNotOrderable(list.__add__, list.__add__)
+        self.assertEqual(list.__add__.__name__, '__add__')
+        self.assertIs(list.__add__.__objclass__, list)
+
+        # Testing objects of <type 'method_descriptor'>...
+        self.assertTrue(list.append == list.append)
+        self.assertFalse(list.append != list.append)
+        self.assertFalse(list.append == list.pop)
+        self.assertTrue(list.append != list.pop)
+        self.assertNotOrderable(list.append, list.append)
+        self.assertEqual(list.append.__name__, 'append')
+        self.assertIs(list.append.__objclass__, list)
 
     def test_not_implemented(self):
         # Testing NotImplemented...
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-06-21-21-42-15.bpo-1617161.tSo2yM.rst b/Misc/NEWS.d/next/Core and Builtins/2018-06-21-21-42-15.bpo-1617161.tSo2yM.rst
new file mode 100644
index 0000000..bd19944
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-06-21-21-42-15.bpo-1617161.tSo2yM.rst
@@ -0,0 +1,7 @@
+The hash of :class:`BuiltinMethodType` instances (methods of built-in classes)
+now depends on the hash of the identity of *__self__* instead of its value.
+The hash and equality of :class:`ModuleType` and :class:`MethodWrapperType`
+instances (methods of user-defined classes and some methods of built-in classes
+like ``str.__add__``) now depend on the hash and equality of the identity
+of *__self__* instead of its value.  :class:`MethodWrapperType` instances no
+longer support ordering.
diff --git a/Objects/classobject.c b/Objects/classobject.c
index 095b0ca..a193ada 100644
--- a/Objects/classobject.c
+++ b/Objects/classobject.c
@@ -225,13 +225,9 @@ method_richcompare(PyObject *self, PyObject *other, int op)
     b = (PyMethodObject *)other;
     eq = PyObject_RichCompareBool(a->im_func, b->im_func, Py_EQ);
     if (eq == 1) {
-        if (a->im_self == NULL || b->im_self == NULL)
-            eq = a->im_self == b->im_self;
-        else
-            eq = PyObject_RichCompareBool(a->im_self, b->im_self,
-                                          Py_EQ);
+        eq = (a->im_self == b->im_self);
     }
-    if (eq < 0)
+    else if (eq < 0)
         return NULL;
     if (op == Py_EQ)
         res = eq ? Py_True : Py_False;
@@ -274,11 +270,9 @@ method_hash(PyMethodObject *a)
 {
     Py_hash_t x, y;
     if (a->im_self == NULL)
-        x = PyObject_Hash(Py_None);
+        x = _Py_HashPointer(Py_None);
     else
-        x = PyObject_Hash(a->im_self);
-    if (x == -1)
-        return -1;
+        x = _Py_HashPointer(a->im_self);
     y = PyObject_Hash(a->im_func);
     if (y == -1)
         return -1;
diff --git a/Objects/descrobject.c b/Objects/descrobject.c
index dfad1ec..b1bee90 100644
--- a/Objects/descrobject.c
+++ b/Objects/descrobject.c
@@ -1038,38 +1038,35 @@ wrapper_dealloc(wrapperobject *wp)
 static PyObject *
 wrapper_richcompare(PyObject *a, PyObject *b, int op)
 {
-    PyWrapperDescrObject *a_descr, *b_descr;
+    wrapperobject *wa, *wb;
+    int eq;
 
     assert(a != NULL && b != NULL);
 
     /* both arguments should be wrapperobjects */
-    if (!Wrapper_Check(a) || !Wrapper_Check(b)) {
+    if ((op != Py_EQ && op != Py_NE)
+        || !Wrapper_Check(a) || !Wrapper_Check(b))
+    {
         Py_RETURN_NOTIMPLEMENTED;
     }
 
-    /* compare by descriptor address; if the descriptors are the same,
-       compare by the objects they're bound to */
-    a_descr = ((wrapperobject *)a)->descr;
-    b_descr = ((wrapperobject *)b)->descr;
-    if (a_descr == b_descr) {
-        a = ((wrapperobject *)a)->self;
-        b = ((wrapperobject *)b)->self;
-        return PyObject_RichCompare(a, b, op);
+    wa = (wrapperobject *)a;
+    wb = (wrapperobject *)b;
+    eq = (wa->descr == wb->descr && wa->self == wb->self);
+    if (eq == (op == Py_EQ)) {
+        Py_RETURN_TRUE;
+    }
+    else {
+        Py_RETURN_FALSE;
     }
-
-    Py_RETURN_RICHCOMPARE(a_descr, b_descr, op);
 }
 
 static Py_hash_t
 wrapper_hash(wrapperobject *wp)
 {
     Py_hash_t x, y;
-    x = _Py_HashPointer(wp->descr);
-    if (x == -1)
-        return -1;
-    y = PyObject_Hash(wp->self);
-    if (y == -1)
-        return -1;
+    x = _Py_HashPointer(wp->self);
+    y = _Py_HashPointer(wp->descr);
     x = x ^ y;
     if (x == -1)
         x = -2;
diff --git a/Objects/methodobject.c b/Objects/methodobject.c
index 9606768..a7042ca 100644
--- a/Objects/methodobject.c
+++ b/Objects/methodobject.c
@@ -251,16 +251,8 @@ static Py_hash_t
 meth_hash(PyCFunctionObject *a)
 {
     Py_hash_t x, y;
-    if (a->m_self == NULL)
-        x = 0;
-    else {
-        x = PyObject_Hash(a->m_self);
-        if (x == -1)
-            return -1;
-    }
+    x = _Py_HashPointer(a->m_self);
     y = _Py_HashPointer((void*)(a->m_ml->ml_meth));
-    if (y == -1)
-        return -1;
     x ^= y;
     if (x == -1)
         x = -2;