Rev 4721: Implement comparison support when using nested StaticTuple objects. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-static-tuple

John Arbash Meinel john at arbash-meinel.com
Wed Sep 30 19:38:00 BST 2009


At http://bazaar.launchpad.net/~jameinel/bzr/2.1-static-tuple

------------------------------------------------------------
revno: 4721
revision-id: john at arbash-meinel.com-20090930183754-p8h2fy6nshpedh8i
parent: john at arbash-meinel.com-20090930180042-jgu7vs8259x2qt71
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-static-tuple
timestamp: Wed 2009-09-30 13:37:54 -0500
message:
  Implement comparison support when using nested StaticTuple objects.
-------------- next part --------------
=== modified file 'bzrlib/_static_tuple_c.c'
--- a/bzrlib/_static_tuple_c.c	2009-09-30 18:00:42 +0000
+++ b/bzrlib/_static_tuple_c.c	2009-09-30 18:37:54 +0000
@@ -156,7 +156,7 @@
     key->flags = 0;
     key->_unused0 = 0;
     key->_unused1 = 0;
-    memset(key->key_bits, 0, sizeof(PyStringObject *) * size);
+    memset(key->key_bits, 0, sizeof(PyObject *) * size);
 #if STATIC_TUPLE_HAS_HASH
     key->hash = -1;
 #endif
@@ -180,10 +180,10 @@
         return NULL;
     }
     len = PyTuple_GET_SIZE(args);
-    if (len <= 0 || len > 256) {
+    if (len < 0 || len > 255) {
         /* Too big or too small */
         PyErr_SetString(PyExc_ValueError, "StaticTuple.__init__(...)"
-            " takes from 1 to 256 key bits");
+            " takes from 0 to 255 key bits");
         return NULL;
     }
     self = (StaticTuple *)(type->tp_alloc(type, len));
@@ -199,9 +199,9 @@
 #endif
     for (i = 0; i < len; ++i) {
         obj = PyTuple_GET_ITEM(args, i);
-        if (!PyString_CheckExact(obj)) {
+        if (!PyString_CheckExact(obj) && !StaticTuple_CheckExact(obj)) {
             PyErr_SetString(PyExc_TypeError, "StaticTuple.__init__(...)"
-                " requires that all key bits are strings.");
+                " requires that all key bits are strings or StaticTuple.");
             /* TODO: What is the proper way to dealloc ? */
             type->tp_dealloc((PyObject *)self);
             return NULL;
@@ -296,10 +296,14 @@
 {
     StaticTuple *vk, *wk;
     Py_ssize_t vlen, wlen, min_len, i;
+    PyObject *v_obj, *w_obj;
     richcmpfunc string_richcompare;
 
     if (!StaticTuple_CheckExact(v)) {
-        /* This has never triggered */
+        /* This has never triggered, according to python-dev it seems this
+         * might trigger if '__op__' is defined but '__rop__' is not, sort of
+         * case. Such as "None == StaticTuple()"
+         */
         fprintf(stderr, "self is tuple\n");
         Py_INCREF(Py_NotImplemented);
         return Py_NotImplemented;
@@ -315,6 +319,9 @@
          */
         return StaticTuple_richcompare_to_tuple(vk, w, op);
     }
+    /* TODO: Py_None is one of the other common cases here, we should probably
+     *       directly support it.
+     */
     if (!StaticTuple_CheckExact(w)) {
         /* Both are not StaticTuple objects, and they aren't Tuple objects or the
          * previous path would have been taken. We don't support comparing with
@@ -326,7 +333,7 @@
     }
     /* Now we know that we have 2 StaticTuple objects, so let's compare them.
      * This code is somewhat borrowed from tuplerichcompare, except we know our
-     * objects are strings, so we get to cheat a bit.
+     * objects are limited in scope, so we cheat a bit.
      */
     if (v == w) {
         /* Identical pointers, we can shortcut this easily. */
@@ -349,12 +356,22 @@
 	min_len = (vlen < wlen) ? vlen : wlen;
     string_richcompare = PyString_Type.tp_richcompare;
     for (i = 0; i < min_len; i++) {
-        PyObject *result;
-        result = string_richcompare((PyObject *)vk->key_bits[i],
-                                    (PyObject *)wk->key_bits[i],
-                                    Py_EQ);
+        PyObject *result = NULL;
+        v_obj = StaticTuple_GET_ITEM(vk, i);
+        w_obj = StaticTuple_GET_ITEM(wk, i);
+        if (PyString_CheckExact(v_obj) && PyString_CheckExact(w_obj)) {
+            result = string_richcompare(v_obj, w_obj, Py_EQ);
+        } else if (StaticTuple_CheckExact(v_obj) &&
+                   StaticTuple_CheckExact(w_obj))
+        {
+            /* Both are StaticTuple types, so recurse */
+            result = StaticTuple_richcompare(v_obj, w_obj, Py_EQ);
+        } else {
+            /* Not the same type, obviously they won't compare equal */
+            break;
+        }
         if (result == NULL) {
-            return NULL; /* Seems to be an error */
+            return NULL; /* There seems to be an error */
         }
         if (result == Py_NotImplemented) {
             PyErr_BadInternalCall();
@@ -362,7 +379,7 @@
             return NULL;
         }
         if (result == Py_False) {
-            /* These strings are not identical
+            /* This entry is not identical
              * Shortcut for Py_EQ
              */
             if (op == Py_EQ) {
@@ -372,8 +389,8 @@
             break;
         }
         if (result != Py_True) {
-            /* We don't know *what* string_richcompare is returning, but it
-             * isn't correct.
+            /* We don't know *what* richcompare is returning, but it
+             * isn't something we recognize
              */
             PyErr_BadInternalCall();
             Py_DECREF(result);
@@ -382,7 +399,9 @@
         Py_DECREF(result);
     }
 	if (i >= vlen || i >= wlen) {
-		/* No more items to compare -- compare sizes */
+        /* We walked off one of the lists, but everything compared equal so
+         * far. Just compare the size.
+         */
 		int cmp;
 		PyObject *res;
 		switch (op) {
@@ -407,9 +426,18 @@
         return Py_True;
     }
     /* It is some other comparison, go ahead and do the real check. */
-    return string_richcompare((PyObject *)vk->key_bits[i],
-                              (PyObject *)wk->key_bits[i],
-                              op);
+    if (PyString_CheckExact(v_obj) && PyString_CheckExact(w_obj))
+    {
+        return string_richcompare(v_obj, w_obj, op);
+    } else if (StaticTuple_CheckExact(v_obj) &&
+               StaticTuple_CheckExact(w_obj))
+    {
+        /* Both are StaticTuple types, so recurse */
+        return StaticTuple_richcompare(v_obj, w_obj, op);
+    } else {
+        Py_INCREF(Py_NotImplemented);
+        return Py_NotImplemented;
+    }
 }
 
 
@@ -501,41 +529,41 @@
 PyTypeObject StaticTuple_Type = {
     PyObject_HEAD_INIT(NULL)
     0,                                           /* ob_size */
-    "StaticTuple",                                       /* tp_name */
-    sizeof(StaticTuple) - sizeof(PyStringObject *),      /* tp_basicsize */
+    "StaticTuple",                               /* tp_name */
+    sizeof(StaticTuple) - sizeof(PyObject *),    /* tp_basicsize */
     sizeof(PyObject *),                          /* tp_itemsize */
-    (destructor)StaticTuple_dealloc,                     /* tp_dealloc */
+    (destructor)StaticTuple_dealloc,             /* tp_dealloc */
     0,                                           /* tp_print */
     0,                                           /* tp_getattr */
     0,                                           /* tp_setattr */
     0,                                           /* tp_compare */
-    (reprfunc)StaticTuple_repr,                          /* tp_repr */
+    (reprfunc)StaticTuple_repr,                  /* tp_repr */
     0,                                           /* tp_as_number */
-    &StaticTuple_as_sequence,                            /* tp_as_sequence */
+    &StaticTuple_as_sequence,                    /* tp_as_sequence */
     0,                                           /* tp_as_mapping */
-    (hashfunc)StaticTuple_hash,                          /* tp_hash */
+    (hashfunc)StaticTuple_hash,                  /* tp_hash */
     0,                                           /* tp_call */
     0,                                           /* tp_str */
     PyObject_GenericGetAttr,                     /* tp_getattro */
     0,                                           /* tp_setattro */
     0,                                           /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT,                          /* tp_flags*/
-    StaticTuple_doc,                                     /* tp_doc */
+    StaticTuple_doc,                             /* tp_doc */
     /* gc.get_referents checks the IS_GC flag before it calls tp_traverse
      * And we don't include this object in the garbage collector because we
      * know it doesn't create cycles. However, 'meliae' will follow
      * tp_traverse, even if the object isn't GC, and we want that.
      */
-    (traverseproc)StaticTuple_traverse,                  /* tp_traverse */
+    (traverseproc)StaticTuple_traverse,          /* tp_traverse */
     0,                                           /* tp_clear */
     // TODO: implement richcompare, we should probably be able to compare vs an
     //       tuple, as well as versus another StaticTuples object.
-    StaticTuple_richcompare,                             /* tp_richcompare */
+    StaticTuple_richcompare,                     /* tp_richcompare */
     0,                                           /* tp_weaklistoffset */
     // We could implement this as returning tuples of keys...
     0,                                           /* tp_iter */
     0,                                           /* tp_iternext */
-    StaticTuple_methods,                                 /* tp_methods */
+    StaticTuple_methods,                         /* tp_methods */
     0,                                           /* tp_members */
     0,                                           /* tp_getset */
     0,                                           /* tp_base */
@@ -545,7 +573,7 @@
     0,                                           /* tp_dictoffset */
     0,                                           /* tp_init */
     0,                                           /* tp_alloc */
-    StaticTuple_new,                                     /* tp_new */
+    StaticTuple_new,                             /* tp_new */
 };
 
 

=== modified file 'bzrlib/_static_tuple_py.py'
--- a/bzrlib/_static_tuple_py.py	2009-09-30 18:00:42 +0000
+++ b/bzrlib/_static_tuple_py.py	2009-09-30 18:37:54 +0000
@@ -29,10 +29,10 @@
     def __init__(self, *args):
         """Create a new 'StaticTuple'"""
         for bit in args:
-            if not isinstance(bit, str):
-                raise TypeError('key bits must be strings')
+            if not isinstance(bit, str) and not isinstance(bit, StaticTuple):
+                raise TypeError('key bits must be strings or StaticTuple')
         num_keys = len(args)
-        if num_keys <= 0 or num_keys > 256:
+        if num_keys < 0 or num_keys > 255:
             raise ValueError('must have 1 => 256 key bits')
         self._tuple = args
 

=== modified file 'bzrlib/tests/test__static_tuple.py'
--- a/bzrlib/tests/test__static_tuple.py	2009-09-30 18:00:42 +0000
+++ b/bzrlib/tests/test__static_tuple.py	2009-09-30 18:37:54 +0000
@@ -83,10 +83,11 @@
         k = self.module.StaticTuple('foo', 'bar')
 
     def test_create_bad_args(self):
-        self.assertRaises(ValueError, self.module.StaticTuple)
-        lots_of_args = ['a']*300
+        args_256 = ['a']*256
         # too many args
-        self.assertRaises(ValueError, self.module.StaticTuple, *lots_of_args)
+        self.assertRaises(ValueError, self.module.StaticTuple, *args_256)
+        args_300 = ['a']*300
+        self.assertRaises(ValueError, self.module.StaticTuple, *args_300)
         # not a string
         self.assertRaises(TypeError, self.module.StaticTuple, 10)
         
@@ -99,12 +100,24 @@
         self.assertEqual(('foo', 'bar'), t)
 
     def test_len(self):
+        k = self.module.StaticTuple()
+        self.assertEqual(0, len(k))
         k = self.module.StaticTuple('foo')
         self.assertEqual(1, len(k))
         k = self.module.StaticTuple('foo', 'bar')
         self.assertEqual(2, len(k))
         k = self.module.StaticTuple('foo', 'bar', 'b', 'b', 'b', 'b', 'b')
         self.assertEqual(7, len(k))
+        args = ['foo']*255
+        k = self.module.StaticTuple(*args)
+        self.assertEqual(255, len(k))
+
+    def test_hold_other_static_tuples(self):
+        k = self.module.StaticTuple('foo', 'bar')
+        k2 = self.module.StaticTuple(k, k)
+        self.assertEqual(2, len(k2))
+        self.assertIs(k, k2[0])
+        self.assertIs(k, k2[1])
 
     def test_getitem(self):
         k = self.module.StaticTuple('foo', 'bar', 'b', 'b', 'b', 'b', 'z')
@@ -145,16 +158,26 @@
     def test_compare_same_obj(self):
         k1 = self.module.StaticTuple('foo', 'bar')
         self.assertCompareEqual(k1, k1)
+        k2 = self.module.StaticTuple(k1, k1)
+        self.assertCompareEqual(k2, k2)
 
     def test_compare_equivalent_obj(self):
         k1 = self.module.StaticTuple('foo', 'bar')
         k2 = self.module.StaticTuple('foo', 'bar')
         self.assertCompareEqual(k1, k2)
+        k3 = self.module.StaticTuple(k1, k2)
+        k4 = self.module.StaticTuple(k2, k1)
+        self.assertCompareEqual(k1, k2)
 
     def test_compare_similar_obj(self):
         k1 = self.module.StaticTuple('foo' + ' bar', 'bar' + ' baz')
         k2 = self.module.StaticTuple('fo' + 'o bar', 'ba' + 'r baz')
         self.assertCompareEqual(k1, k2)
+        k3 = self.module.StaticTuple('foo ' + 'bar', 'bar ' + 'baz')
+        k4 = self.module.StaticTuple('f' + 'oo bar', 'b' + 'ar baz')
+        k5 = self.module.StaticTuple(k1, k2)
+        k6 = self.module.StaticTuple(k3, k4)
+        self.assertCompareEqual(k5, k6)
 
     def assertCompareDifferent(self, k_small, k_big):
         self.assertFalse(k_small == k_big)
@@ -168,16 +191,25 @@
         k1 = self.module.StaticTuple('baz', 'bing')
         k2 = self.module.StaticTuple('foo', 'bar')
         self.assertCompareDifferent(k1, k2)
+        k3 = self.module.StaticTuple(k1, k2)
+        k4 = self.module.StaticTuple(k2, k1)
+        self.assertCompareDifferent(k3, k4)
 
     def test_compare_some_different(self):
         k1 = self.module.StaticTuple('foo', 'bar')
         k2 = self.module.StaticTuple('foo', 'zzz')
         self.assertCompareDifferent(k1, k2)
+        k3 = self.module.StaticTuple(k1, k1)
+        k4 = self.module.StaticTuple(k1, k2)
+        self.assertCompareDifferent(k3, k4)
 
     def test_compare_diff_width(self):
         k1 = self.module.StaticTuple('foo')
         k2 = self.module.StaticTuple('foo', 'bar')
         self.assertCompareDifferent(k1, k2)
+        k3 = self.module.StaticTuple(k1)
+        k4 = self.module.StaticTuple(k1, k2)
+        self.assertCompareDifferent(k3, k4)
 
     def test_compare_to_tuples(self):
         k1 = self.module.StaticTuple('foo')
@@ -195,6 +227,12 @@
         self.assertCompareDifferent(('baz', 'bing'), k2)
         self.assertCompareDifferent(('foo', 10), k2)
 
+        k3 = self.module.StaticTuple(k1, k2)
+        self.assertCompareEqual(k3, (('foo',), ('foo', 'bar')))
+        self.assertCompareEqual((('foo',), ('foo', 'bar')), k3)
+        self.assertCompareEqual(k3, (k1, ('foo', 'bar')))
+        self.assertCompareEqual((k1, ('foo', 'bar')), k3)
+
     def test_hash(self):
         k = self.module.StaticTuple('foo')
         self.assertEqual(hash(k), hash(('foo',)))
@@ -208,6 +246,10 @@
         x[as_tuple] = 'bar'
         self.assertEqual({as_tuple: 'bar'}, x)
 
+        k2 = self.module.StaticTuple(k)
+        as_tuple2 = (('foo', 'bar', 'baz', 'bing'),)
+        self.assertEqual(hash(k2), hash(as_tuple2))
+
     def test_slice(self):
         k = self.module.StaticTuple('foo', 'bar', 'baz', 'bing')
         self.assertEqual(('foo', 'bar'), k[:2])



More information about the bazaar-commits mailing list