[Python-checkins] bpo-1617161: Make the hash and equality of methods not depending on the value of self. (GH-7848)
Serhiy Storchaka
webhook-mailer at python.org
Tue Jul 31 02:18:27 EDT 2018
https://github.com/python/cpython/commit/ac20e0f98d6727ba97a9575bfa2a11b2f6247c35
commit: ac20e0f98d6727ba97a9575bfa2a11b2f6247c35
branch: master
author: Serhiy Storchaka <storchaka at gmail.com>
committer: GitHub <noreply at github.com>
date: 2018-07-31T09:18:24+03:00
summary:
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.
files:
A Misc/NEWS.d/next/Core and Builtins/2018-06-21-21-42-15.bpo-1617161.tSo2yM.rst
M Lib/test/test_class.py
M Lib/test/test_descr.py
M Objects/classobject.c
M Objects/descrobject.c
M Objects/methodobject.c
diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py
index 841cac9171cc..998452a10eee 100644
--- a/Lib/test/test_class.py
+++ b/Lib/test/test_class.py
@@ -534,6 +534,16 @@ class I:
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 @@ def f(self):
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 0e7728ebf2d7..b96d35cc7299 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -4350,38 +4350,71 @@ def __init__(self):
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 000000000000..bd19944075c1
--- /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 095b0cad2b5c..a193ada6d44c 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 dfad1ecda3ca..b1bee904ec86 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 9606768a48c5..a7042ca39e38 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;
More information about the Python-checkins
mailing list