From 53663a695ef2bb96ac0252cd4cc4aa40d4f953be Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Tue, 15 Jul 2008 14:27:37 +0000 Subject: [PATCH] Issue 2235: __hash__ is once again inherited by default, but inheritance can be blocked explicitly so that collections.Hashable remains meaningful --- Include/object.h | 1 + Lib/UserString.py | 6 ++- Lib/decimal.py | 6 +-- Lib/sets.py | 6 +-- Lib/test/seq_tests.py | 3 +- Lib/test/test_descr.py | 8 ++++ Lib/test/test_hash.py | 79 +++++++++++++++++++++++++++++++++++- Lib/test/test_richcmp.py | 65 +---------------------------- Modules/_collectionsmodule.c | 9 +--- Objects/dictobject.c | 2 +- Objects/listobject.c | 2 +- Objects/object.c | 11 +++-- Objects/setobject.c | 2 +- Objects/typeobject.c | 68 +++++++++++-------------------- 14 files changed, 134 insertions(+), 134 deletions(-) diff --git a/Include/object.h b/Include/object.h index e447769915b..c00df74e6e1 100644 --- a/Include/object.h +++ b/Include/object.h @@ -475,6 +475,7 @@ PyAPI_FUNC(PyObject *) PyObject_GenericGetAttr(PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_GenericSetAttr(PyObject *, PyObject *, PyObject *); PyAPI_FUNC(long) PyObject_Hash(PyObject *); +PyAPI_FUNC(long) PyObject_HashNotImplemented(PyObject *); PyAPI_FUNC(int) PyObject_IsTrue(PyObject *); PyAPI_FUNC(int) PyObject_Not(PyObject *); PyAPI_FUNC(int) PyCallable_Check(PyObject *); diff --git a/Lib/UserString.py b/Lib/UserString.py index 446079781a2..726b3f7d3c8 100755 --- a/Lib/UserString.py +++ b/Lib/UserString.py @@ -150,8 +150,10 @@ class MutableString(UserString, collections.MutableSequence): warnpy3k('the class UserString.MutableString has been removed in ' 'Python 3.0', stacklevel=2) self.data = string - def __hash__(self): - raise TypeError, "unhashable type (it is mutable)" + + # We inherit object.__hash__, so we must deny this explicitly + __hash__ = None + def __setitem__(self, index, sub): if isinstance(index, slice): if isinstance(sub, UserString): diff --git a/Lib/decimal.py b/Lib/decimal.py index c94e1be051f..a545cf87c92 100644 --- a/Lib/decimal.py +++ b/Lib/decimal.py @@ -3697,10 +3697,8 @@ class Context(object): for flag in flags: self._ignored_flags.remove(flag) - def __hash__(self): - """A Context cannot be hashed.""" - # We inherit object.__hash__, so we must deny this explicitly - raise TypeError("Cannot hash a Context.") + # We inherit object.__hash__, so we must deny this explicitly + __hash__ = None def Etiny(self): """Returns Etiny (= Emin - prec + 1)""" diff --git a/Lib/sets.py b/Lib/sets.py index 99ee931d9bb..b1743da8447 100644 --- a/Lib/sets.py +++ b/Lib/sets.py @@ -439,10 +439,8 @@ class Set(BaseSet): def __setstate__(self, data): self._data, = data - def __hash__(self): - """A Set cannot be hashed.""" - # We inherit object.__hash__, so we must deny this explicitly - raise TypeError, "Can't hash a Set, only an ImmutableSet." + # We inherit object.__hash__, so we must deny this explicitly + __hash__ = None # In-place union, intersection, differences. # Subtle: The xyz_update() functions deliberately return None, diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py index d9b2d2966e2..c273a3fed95 100644 --- a/Lib/test/seq_tests.py +++ b/Lib/test/seq_tests.py @@ -214,8 +214,7 @@ class CommonTest(unittest.TestCase): # So instances of AllEq must be found in all non-empty sequences. def __eq__(self, other): return True - def __hash__(self): - raise NotImplemented + __hash__ = None # Can't meet hash invariant requirements self.assert_(AllEq() not in self.type2test([])) self.assert_(AllEq() in self.type2test([1])) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 3c607f70cc9..53b7611fb05 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -3283,12 +3283,20 @@ order (MRO) for bases """ self.assertEqual(hash(d), 144) D.__hash__ = lambda self: 100 self.assertEqual(hash(d), 100) + D.__hash__ = None + self.assertRaises(TypeError, hash, d) del D.__hash__ self.assertEqual(hash(d), 144) + B.__hash__ = None + self.assertRaises(TypeError, hash, d) del B.__hash__ self.assertEqual(hash(d), 314) + C.__hash__ = None + self.assertRaises(TypeError, hash, d) del C.__hash__ self.assertEqual(hash(d), 42) + A.__hash__ = None + self.assertRaises(TypeError, hash, d) del A.__hash__ self.assertEqual(hash(d), orig_hash) d.foo = 42 diff --git a/Lib/test/test_hash.py b/Lib/test/test_hash.py index 07b65da9bdf..f3954c2280b 100644 --- a/Lib/test/test_hash.py +++ b/Lib/test/test_hash.py @@ -1,9 +1,11 @@ # test the invariant that # iff a==b then hash(a)==hash(b) # +# Also test that hash implementations are inherited as expected import unittest from test import test_support +from collections import Hashable class HashEqualityTestCase(unittest.TestCase): @@ -39,8 +41,83 @@ class HashEqualityTestCase(unittest.TestCase): self.same_hash(float(0.5), complex(0.5, 0.0)) +_default_hash = object.__hash__ +class DefaultHash(object): pass + +_FIXED_HASH_VALUE = 42 +class FixedHash(object): + def __hash__(self): + return _FIXED_HASH_VALUE + +class OnlyEquality(object): + def __eq__(self, other): + return self is other + +class OnlyInequality(object): + def __ne__(self, other): + return self is not other + +class OnlyCmp(object): + def __cmp__(self, other): + return cmp(id(self), id(other)) + +class InheritedHashWithEquality(FixedHash, OnlyEquality): pass +class InheritedHashWithInequality(FixedHash, OnlyInequality): pass +class InheritedHashWithCmp(FixedHash, OnlyCmp): pass + +class NoHash(object): + __hash__ = None + +class HashInheritanceTestCase(unittest.TestCase): + default_expected = [object(), + DefaultHash(), + ] + fixed_expected = [FixedHash(), + InheritedHashWithEquality(), + InheritedHashWithInequality(), + InheritedHashWithCmp(), + ] + # TODO: Change these to expecting an exception + # when forward porting to Py3k + warning_expected = [OnlyEquality(), + OnlyInequality(), + OnlyCmp(), + ] + error_expected = [NoHash()] + + def test_default_hash(self): + for obj in self.default_expected: + self.assertEqual(hash(obj), _default_hash(obj)) + + def test_fixed_hash(self): + for obj in self.fixed_expected: + self.assertEqual(hash(obj), _FIXED_HASH_VALUE) + + def test_warning_hash(self): + for obj in self.warning_expected: + # TODO: Check for the expected Py3k warning here + obj_hash = hash(obj) + self.assertEqual(obj_hash, _default_hash(obj)) + + def test_error_hash(self): + for obj in self.error_expected: + self.assertRaises(TypeError, hash, obj) + + def test_hashable(self): + objects = (self.default_expected + + self.fixed_expected + + self.warning_expected) + for obj in objects: + self.assert_(isinstance(obj, Hashable), repr(obj)) + + def test_not_hashable(self): + for obj in self.error_expected: + self.assertFalse(isinstance(obj, Hashable), repr(obj)) + + def test_main(): - test_support.run_unittest(HashEqualityTestCase) + test_support.run_unittest(HashEqualityTestCase, + HashInheritanceTestCase) if __name__ == "__main__": diff --git a/Lib/test/test_richcmp.py b/Lib/test/test_richcmp.py index db6d31ff952..ad6838628e6 100644 --- a/Lib/test/test_richcmp.py +++ b/Lib/test/test_richcmp.py @@ -48,8 +48,7 @@ class Vector: def __setitem__(self, i, v): self.data[i] = v - def __hash__(self): - raise TypeError, "Vectors cannot be hashed" + __hash__ = None # Vectors cannot be hashed def __nonzero__(self): raise TypeError, "Vectors cannot be used in Boolean contexts" @@ -85,35 +84,6 @@ class Vector: raise ValueError, "Cannot compare vectors of different length" return other - -class SimpleOrder(object): - """ - A simple class that defines order but not full comparison. - """ - - def __init__(self, value): - self.value = value - - def __lt__(self, other): - if not isinstance(other, SimpleOrder): - return True - return self.value < other.value - - def __gt__(self, other): - if not isinstance(other, SimpleOrder): - return False - return self.value > other.value - - -class DumbEqualityWithoutHash(object): - """ - A class that define __eq__, but no __hash__: it shouldn't be hashable. - """ - - def __eq__(self, other): - return False - - opmap = { "lt": (lambda a,b: a< b, operator.lt, operator.__lt__), "le": (lambda a,b: a<=b, operator.le, operator.__le__), @@ -359,39 +329,8 @@ class ListTest(unittest.TestCase): for op in opmap["lt"]: self.assertIs(op(x, y), True) - -class HashableTest(unittest.TestCase): - """ - Test hashability of classes with rich operators defined. - """ - - def test_simpleOrderHashable(self): - """ - A class that only defines __gt__ and/or __lt__ should be hashable. - """ - a = SimpleOrder(1) - b = SimpleOrder(2) - self.assert_(a < b) - self.assert_(b > a) - self.assert_(a.__hash__ is not None) - - def test_notHashableException(self): - """ - If a class is not hashable, it should raise a TypeError with an - understandable message. - """ - a = DumbEqualityWithoutHash() - try: - hash(a) - except TypeError, e: - self.assertEquals(str(e), - "unhashable type: 'DumbEqualityWithoutHash'") - else: - raise test_support.TestFailed("Should not be here") - - def test_main(): - test_support.run_unittest(VectorTest, NumberTest, MiscTest, DictTest, ListTest, HashableTest) + test_support.run_unittest(VectorTest, NumberTest, MiscTest, DictTest, ListTest) if __name__ == "__main__": test_main() diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 4d13e45540b..18a72f98085 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -608,13 +608,6 @@ deque_traverse(dequeobject *deque, visitproc visit, void *arg) return 0; } -static long -deque_nohash(PyObject *self) -{ - PyErr_SetString(PyExc_TypeError, "deque objects are unhashable"); - return -1; -} - static PyObject * deque_copy(PyObject *deque) { @@ -917,7 +910,7 @@ static PyTypeObject deque_type = { 0, /* tp_as_number */ &deque_as_sequence, /* tp_as_sequence */ 0, /* tp_as_mapping */ - deque_nohash, /* tp_hash */ + (hashfunc)PyObject_HashNotImplemented, /* tp_hash */ 0, /* tp_call */ 0, /* tp_str */ PyObject_GenericGetAttr, /* tp_getattro */ diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 57911654f55..038f3738d65 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2240,7 +2240,7 @@ PyTypeObject PyDict_Type = { 0, /* tp_as_number */ &dict_as_sequence, /* tp_as_sequence */ &dict_as_mapping, /* tp_as_mapping */ - 0, /* tp_hash */ + (hashfunc)PyObject_HashNotImplemented, /* tp_hash */ 0, /* tp_call */ 0, /* tp_str */ PyObject_GenericGetAttr, /* tp_getattro */ diff --git a/Objects/listobject.c b/Objects/listobject.c index 16a2ce62c62..10f2c5da259 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -2742,7 +2742,7 @@ PyTypeObject PyList_Type = { 0, /* tp_as_number */ &list_as_sequence, /* tp_as_sequence */ &list_as_mapping, /* tp_as_mapping */ - 0, /* tp_hash */ + (hashfunc)PyObject_HashNotImplemented, /* tp_hash */ 0, /* tp_call */ 0, /* tp_str */ PyObject_GenericGetAttr, /* tp_getattro */ diff --git a/Objects/object.c b/Objects/object.c index f40fd9f5878..9cd34b8d4e3 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1083,6 +1083,13 @@ finally: #endif } +long +PyObject_HashNotImplemented(PyObject *self) +{ + PyErr_Format(PyExc_TypeError, "unhashable type: '%.200s'", + self->ob_type->tp_name); + return -1; +} long PyObject_Hash(PyObject *v) @@ -1094,9 +1101,7 @@ PyObject_Hash(PyObject *v) return _Py_HashPointer(v); /* Use address as hash value */ } /* If there's a cmp but no hash defined, the object can't be hashed */ - PyErr_Format(PyExc_TypeError, "unhashable type: '%.200s'", - v->ob_type->tp_name); - return -1; + return PyObject_HashNotImplemented(v); } PyObject * diff --git a/Objects/setobject.c b/Objects/setobject.c index 39465f32860..fbbdf6ede39 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2109,7 +2109,7 @@ PyTypeObject PySet_Type = { &set_as_number, /* tp_as_number */ &set_as_sequence, /* tp_as_sequence */ 0, /* tp_as_mapping */ - 0, /* tp_hash */ + (hashfunc)PyObject_HashNotImplemented, /* tp_hash */ 0, /* tp_call */ 0, /* tp_str */ PyObject_GenericGetAttr, /* tp_getattro */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e0ae55b057c..0af3f30de5e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3648,27 +3648,6 @@ inherit_special(PyTypeObject *type, PyTypeObject *base) type->tp_flags |= Py_TPFLAGS_DICT_SUBCLASS; } -static char *hash_name_op[] = { - "__eq__", - "__cmp__", - "__hash__", - NULL -}; - -static int -overrides_hash(PyTypeObject *type) -{ - char **p; - PyObject *dict = type->tp_dict; - - assert(dict != NULL); - for (p = hash_name_op; *p; p++) { - if (PyDict_GetItemString(dict, *p) != NULL) - return 1; - } - return 0; -} - static void inherit_slots(PyTypeObject *type, PyTypeObject *base) { @@ -3802,8 +3781,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base) if (type->tp_flags & base->tp_flags & Py_TPFLAGS_HAVE_RICHCOMPARE) { if (type->tp_compare == NULL && type->tp_richcompare == NULL && - type->tp_hash == NULL && - !overrides_hash(type)) + type->tp_hash == NULL) { type->tp_compare = base->tp_compare; type->tp_richcompare = base->tp_richcompare; @@ -3984,18 +3962,6 @@ PyType_Ready(PyTypeObject *type) } } - /* Hack for tp_hash and __hash__. - If after all that, tp_hash is still NULL, and __hash__ is not in - tp_dict, set tp_dict['__hash__'] equal to None. - This signals that __hash__ is not inherited. - */ - if (type->tp_hash == NULL && - PyDict_GetItemString(type->tp_dict, "__hash__") == NULL && - PyDict_SetItemString(type->tp_dict, "__hash__", Py_None) < 0) - { - goto error; - } - /* Some more special stuff */ base = type->tp_base; if (base != NULL) { @@ -5280,10 +5246,8 @@ slot_tp_hash(PyObject *self) func = lookup_method(self, "__cmp__", &cmp_str); } if (func != NULL) { - PyErr_Format(PyExc_TypeError, "unhashable type: '%.200s'", - self->ob_type->tp_name); Py_DECREF(func); - return -1; + return PyObject_HashNotImplemented(self); } PyErr_Clear(); h = _Py_HashPointer((void *)self); @@ -6034,6 +5998,13 @@ update_one_slot(PyTypeObject *type, slotdef *p) sanity checks. I'll buy the first person to point out a bug in this reasoning a beer. */ } + else if (descr == Py_None && + strcmp(p->name, "__hash__") == 0) { + /* We specifically allow __hash__ to be set to None + to prevent inheritance of the default + implementation from object.__hash__ */ + specific = PyObject_HashNotImplemented; + } else { use_generic = 1; generic = p->function; @@ -6247,12 +6218,21 @@ add_operators(PyTypeObject *type) continue; if (PyDict_GetItem(dict, p->name_strobj)) continue; - descr = PyDescr_NewWrapper(type, p, *ptr); - if (descr == NULL) - return -1; - if (PyDict_SetItem(dict, p->name_strobj, descr) < 0) - return -1; - Py_DECREF(descr); + if (*ptr == PyObject_HashNotImplemented) { + /* Classes may prevent the inheritance of the tp_hash + slot by storing PyObject_HashNotImplemented in it. Make it + visible as a None value for the __hash__ attribute. */ + if (PyDict_SetItem(dict, p->name_strobj, Py_None) < 0) + return -1; + } + else { + descr = PyDescr_NewWrapper(type, p, *ptr); + if (descr == NULL) + return -1; + if (PyDict_SetItem(dict, p->name_strobj, descr) < 0) + return -1; + Py_DECREF(descr); + } } if (type->tp_new != NULL) { if (add_tp_new_wrapper(type) < 0)