Rev 4746: (jam) Update SimpleSet to properly handle NotImplemented in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4746 [merge]
revision-id: pqm at pqm.ubuntu.com-20091015013646-k4nv3t54nby3tmnq
parent: pqm at pqm.ubuntu.com-20091014085144-t3bcnsw7rvm7b6ge
parent: john at arbash-meinel.com-20091014155706-zfqfohnjfido8umi
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-10-15 02:36:46 +0100
message:
  (jam) Update SimpleSet to properly handle NotImplemented
  	richcomparisons.
modified:
  bzrlib/_simple_set_pyx.pyx     _static_tuple_intern-20091002053806-sid67p8spedru51w-1
  bzrlib/tests/test__simple_set.py test__static_tuple_i-20091002053806-sid67p8spedru51w-2
=== modified file 'bzrlib/_simple_set_pyx.pyx'
--- a/bzrlib/_simple_set_pyx.pyx	2009-10-13 16:44:43 +0000
+++ b/bzrlib/_simple_set_pyx.pyx	2009-10-14 15:57:06 +0000
@@ -21,13 +21,11 @@
 
 cdef extern from "Python.h":
     ctypedef unsigned long size_t
-    ctypedef long (*hashfunc)(PyObject*)
-    ctypedef PyObject *(*richcmpfunc)(PyObject *, PyObject *, int)
+    ctypedef long (*hashfunc)(PyObject*) except -1
+    ctypedef object (*richcmpfunc)(PyObject *, PyObject *, int)
     ctypedef int (*visitproc)(PyObject *, void *)
     ctypedef int (*traverseproc)(PyObject *, visitproc, void *)
     int Py_EQ
-    PyObject *Py_True
-    PyObject *Py_NotImplemented
     void Py_INCREF(PyObject *)
     void Py_DECREF(PyObject *)
     ctypedef struct PyTypeObject:
@@ -36,7 +34,6 @@
         traverseproc tp_traverse
 
     PyTypeObject *Py_TYPE(PyObject *)
-    int PyObject_IsTrue(PyObject *)
     # Note: *Don't* use hash(), Pyrex 0.9.8.5 thinks it returns an 'int', and
     #       thus silently truncates to 32-bits on 64-bit machines.
     long PyObject_Hash(PyObject *) except -1
@@ -58,9 +55,12 @@
 _dummy = <PyObject *>_dummy_obj
 
 
+cdef object _NotImplemented
+_NotImplemented = NotImplemented
+
+
 cdef int _is_equal(PyObject *this, long this_hash, PyObject *other) except -1:
     cdef long other_hash
-    cdef PyObject *res
 
     if this == other:
         return 1
@@ -76,20 +76,12 @@
     #      equal. (It doesn't try to cast them both to some intermediate form
     #      that would compare equal.)
     res = Py_TYPE(this).tp_richcompare(this, other, Py_EQ)
-    if res == NULL: # Exception
-        return -1
-    if PyObject_IsTrue(res):
-        Py_DECREF(res)
-        return 1
-    if res == Py_NotImplemented:
-        Py_DECREF(res)
+    if res is _NotImplemented:
         res = Py_TYPE(other).tp_richcompare(other, this, Py_EQ)
-    if res == NULL:
-        return -1
-    if PyObject_IsTrue(res):
-        Py_DECREF(res)
+        if res is _NotImplemented:
+            return 0
+    if res:
         return 1
-    Py_DECREF(res)
     return 0
 
 

=== modified file 'bzrlib/tests/test__simple_set.py'
--- a/bzrlib/tests/test__simple_set.py	2009-10-12 15:55:26 +0000
+++ b/bzrlib/tests/test__simple_set.py	2009-10-14 15:53:34 +0000
@@ -69,6 +69,12 @@
         raise RuntimeError('I refuse to play nice')
 
 
+class _NoImplementCompare(_Hashable):
+
+    def __eq__(self, other):
+        return NotImplemented
+
+
 # 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
 
@@ -329,6 +335,20 @@
         # Tries to compare with k1, fails
         self.assertRaises(RuntimeError, obj.add, k2)
 
+    def test_richcompare_not_implemented(self):
+        obj = self.module.SimpleSet()
+        # Even though their hashes are the same, tp_richcompare returns
+        # NotImplemented, which means we treat them as not equal
+        k1 = _NoImplementCompare(200)
+        k2 = _NoImplementCompare(200)
+        self.assertLookup(200, '<null>', obj, k1)
+        self.assertLookup(200, '<null>', obj, k2)
+        self.assertIs(k1, obj.add(k1))
+        self.assertLookup(200, k1, obj, k1)
+        self.assertLookup(201, '<null>', obj, k2)
+        self.assertIs(k2, obj.add(k2))
+        self.assertIs(k1, obj[k1])
+
     def test_add_and_remove_lots_of_items(self):
         obj = self.module.SimpleSet()
         chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890'