Rev 4769: A bit more error checking in _is_equal. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-simple-set

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.1-simple-set

------------------------------------------------------------
revno: 4769
revision-id: john at arbash-meinel.com-20091012145850-9363u41ovkaz7nka
parent: john at arbash-meinel.com-20091009170327-ezw4vx4su1msq3ap
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-simple-set
timestamp: Mon 2009-10-12 09:58:50 -0500
message:
  A bit more error checking in _is_equal.
  
  Includes test cases that cover some bad-behavior in classes.
-------------- next part --------------
=== modified file 'bzrlib/_simple_set_pyx.pyx'
--- a/bzrlib/_simple_set_pyx.pyx	2009-10-09 16:49:45 +0000
+++ b/bzrlib/_simple_set_pyx.pyx	2009-10-12 14:58:50 +0000
@@ -59,6 +59,10 @@
     if this == other:
         return 1
     other_hash = Py_TYPE(other).tp_hash(other)
+    if other_hash == -1:
+        # Even though other successfully hashed in the past, it seems to have
+        # changed its mind, and failed this time, so propogate the failure.
+        return -1
     if other_hash != this_hash:
         return 0
 
@@ -426,7 +430,6 @@
     cdef size_t i, perturb
     cdef Py_ssize_t mask
     cdef long key_hash
-    cdef long this_hash
     cdef PyObject **table, **slot, *cur, **free_slot, *py_key
 
     key_hash = hash(key)

=== modified file 'bzrlib/tests/test__simple_set.py'
--- a/bzrlib/tests/test__simple_set.py	2009-10-09 17:03:27 +0000
+++ b/bzrlib/tests/test__simple_set.py	2009-10-12 14:58:50 +0000
@@ -48,6 +48,27 @@
             return NotImplemented
         return other.hash == self.hash
 
+
+class _BadSecondHash(_Hashable):
+
+    def __init__(self, the_hash):
+        _Hashable.__init__(self, the_hash)
+        self._first = True
+
+    def __hash__(self):
+        if self._first:
+            self._first = False
+            return self.hash
+        else:
+            raise ValueError('I can only be hashed once.')
+
+
+class _BadCompare(_Hashable):
+
+    def __eq__(self, other):
+        raise RuntimeError('I refuse to play nice')
+
+
 # Even though this is an extension, we don't permute the tests for a python
 # version. As the plain python version is just a dict or set
 
@@ -258,6 +279,23 @@
         self.assertFillState(1, 1, 0x7ff, obj)
         self.assertEqual((591, '<null>'), obj._test_lookup(k2))
 
+    def test_second_hash_failure(self):
+        obj = self.module.SimpleSet()
+        k1 = _BadSecondHash(200)
+        k2 = _Hashable(200)
+        # Should only call hash() one time
+        obj.add(k1)
+        self.assertFalse(k1._first)
+        self.assertRaises(ValueError, obj.add, k2)
+
+    def test_richcompare_failure(self):
+        obj = self.module.SimpleSet()
+        k1 = _Hashable(200)
+        k2 = _BadCompare(200)
+        obj.add(k1)
+        # Tries to compare with k1, fails
+        self.assertRaises(RuntimeError, obj.add, k2)
+
     def test_add_and_remove_lots_of_items(self):
         obj = self.module.SimpleSet()
         chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890'