Skip to content

Commit ac20e0f

Browse files
serhiy-storchakaserhiy-storchaka
authored
bpo-1617161: Make the hash and equality of methods not depending on the value of self. (pythonGH-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.
1 parent c48e26d commit ac20e0f

6 files changed

Lines changed: 112 additions & 73 deletions

File tree

‎Lib/test/test_class.py‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,16 @@ class I:
534534
else:
535535
self.fail("attribute error for I.__init__ got masked")
536536

537+
def assertNotOrderable(self, a, b):
538+
with self.assertRaises(TypeError):
539+
a < b
540+
with self.assertRaises(TypeError):
541+
a > b
542+
with self.assertRaises(TypeError):
543+
a <= b
544+
with self.assertRaises(TypeError):
545+
a >= b
546+
537547
def testHashComparisonOfMethods(self):
538548
# Test comparison and hash of methods
539549
class A:
@@ -544,24 +554,30 @@ def f(self):
544554
def g(self):
545555
pass
546556
def __eq__(self, other):
547-
return self.x == other.x
557+
return True
548558
def __hash__(self):
549-
return self.x
559+
raise TypeError
550560
class B(A):
551561
pass
552562

553563
a1 = A(1)
554-
a2 = A(2)
555-
self.assertEqual(a1.f, a1.f)
556-
self.assertNotEqual(a1.f, a2.f)
557-
self.assertNotEqual(a1.f, a1.g)
558-
self.assertEqual(a1.f, A(1).f)
564+
a2 = A(1)
565+
self.assertTrue(a1.f == a1.f)
566+
self.assertFalse(a1.f != a1.f)
567+
self.assertFalse(a1.f == a2.f)
568+
self.assertTrue(a1.f != a2.f)
569+
self.assertFalse(a1.f == a1.g)
570+
self.assertTrue(a1.f != a1.g)
571+
self.assertNotOrderable(a1.f, a1.f)
559572
self.assertEqual(hash(a1.f), hash(a1.f))
560-
self.assertEqual(hash(a1.f), hash(A(1).f))
561573

562-
self.assertNotEqual(A.f, a1.f)
563-
self.assertNotEqual(A.f, A.g)
564-
self.assertEqual(B.f, A.f)
574+
self.assertFalse(A.f == a1.f)
575+
self.assertTrue(A.f != a1.f)
576+
self.assertFalse(A.f == A.g)
577+
self.assertTrue(A.f != A.g)
578+
self.assertTrue(B.f == A.f)
579+
self.assertFalse(B.f != A.f)
580+
self.assertNotOrderable(A.f, A.f)
565581
self.assertEqual(hash(B.f), hash(A.f))
566582

567583
# the following triggers a SystemError in 2.4

‎Lib/test/test_descr.py‎

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4350,38 +4350,71 @@ def __init__(self):
43504350
else:
43514351
self.fail("did not test __init__() for None return")
43524352

4353+
def assertNotOrderable(self, a, b):
4354+
with self.assertRaises(TypeError):
4355+
a < b
4356+
with self.assertRaises(TypeError):
4357+
a > b
4358+
with self.assertRaises(TypeError):
4359+
a <= b
4360+
with self.assertRaises(TypeError):
4361+
a >= b
4362+
43534363
def test_method_wrapper(self):
43544364
# Testing method-wrapper objects...
43554365
# <type 'method-wrapper'> did not support any reflection before 2.5
4356-
4357-
# XXX should methods really support __eq__?
4358-
43594366
l = []
4360-
self.assertEqual(l.__add__, l.__add__)
4361-
self.assertEqual(l.__add__, [].__add__)
4362-
self.assertNotEqual(l.__add__, [5].__add__)
4363-
self.assertNotEqual(l.__add__, l.__mul__)
4367+
self.assertTrue(l.__add__ == l.__add__)
4368+
self.assertFalse(l.__add__ != l.__add__)
4369+
self.assertFalse(l.__add__ == [].__add__)
4370+
self.assertTrue(l.__add__ != [].__add__)
4371+
self.assertFalse(l.__add__ == l.__mul__)
4372+
self.assertTrue(l.__add__ != l.__mul__)
4373+
self.assertNotOrderable(l.__add__, l.__add__)
43644374
self.assertEqual(l.__add__.__name__, '__add__')
4365-
if hasattr(l.__add__, '__self__'):
4366-
# CPython
4367-
self.assertIs(l.__add__.__self__, l)
4368-
self.assertIs(l.__add__.__objclass__, list)
4369-
else:
4370-
# Python implementations where [].__add__ is a normal bound method
4371-
self.assertIs(l.__add__.im_self, l)
4372-
self.assertIs(l.__add__.im_class, list)
4375+
self.assertIs(l.__add__.__self__, l)
4376+
self.assertIs(l.__add__.__objclass__, list)
43734377
self.assertEqual(l.__add__.__doc__, list.__add__.__doc__)
4374-
try:
4375-
hash(l.__add__)
4376-
except TypeError:
4377-
pass
4378-
else:
4379-
self.fail("no TypeError from hash([].__add__)")
4378+
# hash([].__add__) should not be based on hash([])
4379+
hash(l.__add__)
43804380

4381-
t = ()
4382-
t += (7,)
4383-
self.assertEqual(t.__add__, (7,).__add__)
4384-
self.assertEqual(hash(t.__add__), hash((7,).__add__))
4381+
def test_builtin_function_or_method(self):
4382+
# Not really belonging to test_descr, but introspection and
4383+
# comparison on <type 'builtin_function_or_method'> seems not
4384+
# to be tested elsewhere
4385+
l = []
4386+
self.assertTrue(l.append == l.append)
4387+
self.assertFalse(l.append != l.append)
4388+
self.assertFalse(l.append == [].append)
4389+
self.assertTrue(l.append != [].append)
4390+
self.assertFalse(l.append == l.pop)
4391+
self.assertTrue(l.append != l.pop)
4392+
self.assertNotOrderable(l.append, l.append)
4393+
self.assertEqual(l.append.__name__, 'append')
4394+
self.assertIs(l.append.__self__, l)
4395+
# self.assertIs(l.append.__objclass__, list) --- could be added?
4396+
self.assertEqual(l.append.__doc__, list.append.__doc__)
4397+
# hash([].append) should not be based on hash([])
4398+
hash(l.append)
4399+
4400+
def test_special_unbound_method_types(self):
4401+
# Testing objects of <type 'wrapper_descriptor'>...
4402+
self.assertTrue(list.__add__ == list.__add__)
4403+
self.assertFalse(list.__add__ != list.__add__)
4404+
self.assertFalse(list.__add__ == list.__mul__)
4405+
self.assertTrue(list.__add__ != list.__mul__)
4406+
self.assertNotOrderable(list.__add__, list.__add__)
4407+
self.assertEqual(list.__add__.__name__, '__add__')
4408+
self.assertIs(list.__add__.__objclass__, list)
4409+
4410+
# Testing objects of <type 'method_descriptor'>...
4411+
self.assertTrue(list.append == list.append)
4412+
self.assertFalse(list.append != list.append)
4413+
self.assertFalse(list.append == list.pop)
4414+
self.assertTrue(list.append != list.pop)
4415+
self.assertNotOrderable(list.append, list.append)
4416+
self.assertEqual(list.append.__name__, 'append')
4417+
self.assertIs(list.append.__objclass__, list)
43854418

43864419
def test_not_implemented(self):
43874420
# Testing NotImplemented...

‎Misc/NEWS.d/next/Core and Builtins/2018-06-21-21-42-15.bpo-1617161.tSo2yM.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
The hash of :class:`BuiltinMethodType` instances (methods of built-in classes)
2+
now depends on the hash of the identity of *__self__* instead of its value.
3+
The hash and equality of :class:`ModuleType` and :class:`MethodWrapperType`
4+
instances (methods of user-defined classes and some methods of built-in classes
5+
like ``str.__add__``) now depend on the hash and equality of the identity
6+
of *__self__* instead of its value. :class:`MethodWrapperType` instances no
7+
longer support ordering.

‎Objects/classobject.c‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,9 @@ method_richcompare(PyObject *self, PyObject *other, int op)
225225
b = (PyMethodObject *)other;
226226
eq = PyObject_RichCompareBool(a->im_func, b->im_func, Py_EQ);
227227
if (eq == 1) {
228-
if (a->im_self == NULL || b->im_self == NULL)
229-
eq = a->im_self == b->im_self;
230-
else
231-
eq = PyObject_RichCompareBool(a->im_self, b->im_self,
232-
Py_EQ);
228+
eq = (a->im_self == b->im_self);
233229
}
234-
if (eq < 0)
230+
else if (eq < 0)
235231
return NULL;
236232
if (op == Py_EQ)
237233
res = eq ? Py_True : Py_False;
@@ -274,11 +270,9 @@ method_hash(PyMethodObject *a)
274270
{
275271
Py_hash_t x, y;
276272
if (a->im_self == NULL)
277-
x = PyObject_Hash(Py_None);
273+
x = _Py_HashPointer(Py_None);
278274
else
279-
x = PyObject_Hash(a->im_self);
280-
if (x == -1)
281-
return -1;
275+
x = _Py_HashPointer(a->im_self);
282276
y = PyObject_Hash(a->im_func);
283277
if (y == -1)
284278
return -1;

‎Objects/descrobject.c‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,38 +1038,35 @@ wrapper_dealloc(wrapperobject *wp)
10381038
static PyObject *
10391039
wrapper_richcompare(PyObject *a, PyObject *b, int op)
10401040
{
1041-
PyWrapperDescrObject *a_descr, *b_descr;
1041+
wrapperobject *wa, *wb;
1042+
int eq;
10421043

10431044
assert(a != NULL && b != NULL);
10441045

10451046
/* both arguments should be wrapperobjects */
1046-
if (!Wrapper_Check(a) || !Wrapper_Check(b)) {
1047+
if ((op != Py_EQ && op != Py_NE)
1048+
|| !Wrapper_Check(a) || !Wrapper_Check(b))
1049+
{
10471050
Py_RETURN_NOTIMPLEMENTED;
10481051
}
10491052

1050-
/* compare by descriptor address; if the descriptors are the same,
1051-
compare by the objects they're bound to */
1052-
a_descr = ((wrapperobject *)a)->descr;
1053-
b_descr = ((wrapperobject *)b)->descr;
1054-
if (a_descr == b_descr) {
1055-
a = ((wrapperobject *)a)->self;
1056-
b = ((wrapperobject *)b)->self;
1057-
return PyObject_RichCompare(a, b, op);
1053+
wa = (wrapperobject *)a;
1054+
wb = (wrapperobject *)b;
1055+
eq = (wa->descr == wb->descr && wa->self == wb->self);
1056+
if (eq == (op == Py_EQ)) {
1057+
Py_RETURN_TRUE;
1058+
}
1059+
else {
1060+
Py_RETURN_FALSE;
10581061
}
1059-
1060-
Py_RETURN_RICHCOMPARE(a_descr, b_descr, op);
10611062
}
10621063

10631064
static Py_hash_t
10641065
wrapper_hash(wrapperobject *wp)
10651066
{
10661067
Py_hash_t x, y;
1067-
x = _Py_HashPointer(wp->descr);
1068-
if (x == -1)
1069-
return -1;
1070-
y = PyObject_Hash(wp->self);
1071-
if (y == -1)
1072-
return -1;
1068+
x = _Py_HashPointer(wp->self);
1069+
y = _Py_HashPointer(wp->descr);
10731070
x = x ^ y;
10741071
if (x == -1)
10751072
x = -2;

‎Objects/methodobject.c‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -251,16 +251,8 @@ static Py_hash_t
251251
meth_hash(PyCFunctionObject *a)
252252
{
253253
Py_hash_t x, y;
254-
if (a->m_self == NULL)
255-
x = 0;
256-
else {
257-
x = PyObject_Hash(a->m_self);
258-
if (x == -1)
259-
return -1;
260-
}
254+
x = _Py_HashPointer(a->m_self);
261255
y = _Py_HashPointer((void*)(a->m_ml->ml_meth));
262-
if (y == -1)
263-
return -1;
264256
x ^= y;
265257
if (x == -1)
266258
x = -2;

0 commit comments

Comments
 (0)