[Python-checkins] r64962 - in python/trunk: Include/object.h Lib/UserString.py Lib/decimal.py Lib/sets.py Lib/test/seq_tests.py Lib/test/test_descr.py Lib/test/test_hash.py Lib/test/test_richcmp.py Modules/_collectionsmodule.c Objects/dictobject.c Objects/listobject.c Objects/object.c Objects/setobject.c Objects/typeobject.c

nick.coghlan python-checkins at python.org
Tue Jul 15 16:27:38 CEST 2008


Author: nick.coghlan
Date: Tue Jul 15 16:27:37 2008
New Revision: 64962

Log:
Issue 2235: __hash__ is once again inherited by default, but inheritance can be blocked explicitly so that collections.Hashable remains meaningful

Modified:
   python/trunk/Include/object.h
   python/trunk/Lib/UserString.py
   python/trunk/Lib/decimal.py
   python/trunk/Lib/sets.py
   python/trunk/Lib/test/seq_tests.py
   python/trunk/Lib/test/test_descr.py
   python/trunk/Lib/test/test_hash.py
   python/trunk/Lib/test/test_richcmp.py
   python/trunk/Modules/_collectionsmodule.c
   python/trunk/Objects/dictobject.c
   python/trunk/Objects/listobject.c
   python/trunk/Objects/object.c
   python/trunk/Objects/setobject.c
   python/trunk/Objects/typeobject.c

Modified: python/trunk/Include/object.h
==============================================================================
--- python/trunk/Include/object.h	(original)
+++ python/trunk/Include/object.h	Tue Jul 15 16:27:37 2008
@@ -475,6 +475,7 @@
 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 *);

Modified: python/trunk/Lib/UserString.py
==============================================================================
--- python/trunk/Lib/UserString.py	(original)
+++ python/trunk/Lib/UserString.py	Tue Jul 15 16:27:37 2008
@@ -150,8 +150,10 @@
         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):

Modified: python/trunk/Lib/decimal.py
==============================================================================
--- python/trunk/Lib/decimal.py	(original)
+++ python/trunk/Lib/decimal.py	Tue Jul 15 16:27:37 2008
@@ -3697,10 +3697,8 @@
         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)"""

Modified: python/trunk/Lib/sets.py
==============================================================================
--- python/trunk/Lib/sets.py	(original)
+++ python/trunk/Lib/sets.py	Tue Jul 15 16:27:37 2008
@@ -439,10 +439,8 @@
     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,

Modified: python/trunk/Lib/test/seq_tests.py
==============================================================================
--- python/trunk/Lib/test/seq_tests.py	(original)
+++ python/trunk/Lib/test/seq_tests.py	Tue Jul 15 16:27:37 2008
@@ -214,8 +214,7 @@
             # 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]))
 

Modified: python/trunk/Lib/test/test_descr.py
==============================================================================
--- python/trunk/Lib/test/test_descr.py	(original)
+++ python/trunk/Lib/test/test_descr.py	Tue Jul 15 16:27:37 2008
@@ -3283,12 +3283,20 @@
         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

Modified: python/trunk/Lib/test/test_hash.py
==============================================================================
--- python/trunk/Lib/test/test_hash.py	(original)
+++ python/trunk/Lib/test/test_hash.py	Tue Jul 15 16:27:37 2008
@@ -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 @@
         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__":

Modified: python/trunk/Lib/test/test_richcmp.py
==============================================================================
--- python/trunk/Lib/test/test_richcmp.py	(original)
+++ python/trunk/Lib/test/test_richcmp.py	Tue Jul 15 16:27:37 2008
@@ -48,8 +48,7 @@
     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 @@
             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 @@
         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()

Modified: python/trunk/Modules/_collectionsmodule.c
==============================================================================
--- python/trunk/Modules/_collectionsmodule.c	(original)
+++ python/trunk/Modules/_collectionsmodule.c	Tue Jul 15 16:27:37 2008
@@ -608,13 +608,6 @@
 	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 @@
 	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 */

Modified: python/trunk/Objects/dictobject.c
==============================================================================
--- python/trunk/Objects/dictobject.c	(original)
+++ python/trunk/Objects/dictobject.c	Tue Jul 15 16:27:37 2008
@@ -2240,7 +2240,7 @@
 	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 */

Modified: python/trunk/Objects/listobject.c
==============================================================================
--- python/trunk/Objects/listobject.c	(original)
+++ python/trunk/Objects/listobject.c	Tue Jul 15 16:27:37 2008
@@ -2742,7 +2742,7 @@
 	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 */

Modified: python/trunk/Objects/object.c
==============================================================================
--- python/trunk/Objects/object.c	(original)
+++ python/trunk/Objects/object.c	Tue Jul 15 16:27:37 2008
@@ -1083,6 +1083,13 @@
 #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 @@
 		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 *

Modified: python/trunk/Objects/setobject.c
==============================================================================
--- python/trunk/Objects/setobject.c	(original)
+++ python/trunk/Objects/setobject.c	Tue Jul 15 16:27:37 2008
@@ -2109,7 +2109,7 @@
 	&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 */

Modified: python/trunk/Objects/typeobject.c
==============================================================================
--- python/trunk/Objects/typeobject.c	(original)
+++ python/trunk/Objects/typeobject.c	Tue Jul 15 16:27:37 2008
@@ -3648,27 +3648,6 @@
 		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 @@
 	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 @@
 		}
 	}
 
-	/* 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 @@
 			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 @@
 			   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 @@
 			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)


More information about the Python-checkins mailing list