Rich comparisons:

- Use PyObject_RichCompareBool() when comparing keys; this makes the
  error handling cleaner.

- There were two implementations for dictionary comparison, an old one
  (#ifdef'ed out) and a new one.  Got rid of the old one, which was
  abandoned years ago.

- In the characterize() function, part of dictionary comparison, use
  PyObject_RichCompareBool() to compare keys and values instead.  But
  continue to use PyObject_Compare() for comparing the final
  (deciding) elements.

- Align the comments in the type struct initializer.

Note: I don't implement rich comparison for dictionaries -- there
doesn't seem to be much to be gained.  (The existing comparison
already decides that shorter dicts are always smaller than longer
dicts.)

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 9398d0dbffc4544d8f15e2cd3e70549a1f168ac1..ad8ba199973e6ddf88b322ff3580a29e9c4a4418 100644 (file)
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -207,15 +207,15 @@ lookdict(dictobject *mp, PyObject *key, register long hash)
                                restore_error = 1;
                                PyErr_Fetch(&err_type, &err_value, &err_tb);
                        }
-                       cmp = PyObject_Compare(ep->me_key, key);
-                       if (PyErr_Occurred())
-                               PyErr_Clear();
-                       else if (cmp == 0) {
+                       cmp = PyObject_RichCompareBool(ep->me_key, key, Py_EQ);
+                       if (cmp > 0) {
                                if (restore_error)
                                        PyErr_Restore(err_type, err_value,
                                                      err_tb);
                                return ep;
                        }
+                       else if (cmp < 0)
+                               PyErr_Clear();
                }
                freeslot = NULL;
        }
@@ -252,15 +252,15 @@ lookdict(dictobject *mp, PyObject *key, register long hash)
                                                    &err_tb);
                                }
                        }
-                       cmp = PyObject_Compare(ep->me_key, key);
-                       if (PyErr_Occurred())
-                               PyErr_Clear();
-                       else if (cmp == 0) {
+                       cmp = PyObject_RichCompareBool(ep->me_key, key, Py_EQ);
+                       if (cmp > 0) {
                                if (restore_error)
                                        PyErr_Restore(err_type, err_value,
                                                      err_tb);
                                return ep;
                        }
+                       else if (cmp < 0)
+                               PyErr_Clear();
                }
                /* Cycle through GF(2^n)-{0} */
                incr = incr << 1;
@@ -912,10 +912,6 @@ PyDict_Items(PyObject *mp)
        return dict_items((dictobject *)mp, (PyObject *)NULL);
 }
 
-#define NEWCMP
-
-#ifdef NEWCMP
-
 /* Subroutine which returns the smallest key in a for which b's value
    is different or absent.  The value is returned too, through the
    pval argument.  No reference counts are incremented. */
@@ -924,20 +920,30 @@ static PyObject *
 characterize(dictobject *a, dictobject *b, PyObject **pval)
 {
        PyObject *diff = NULL;
-       int i;
+       int i, cmp;
 
        *pval = NULL;
        for (i = 0; i < a->ma_size; i++) {
                if (a->ma_table[i].me_value != NULL) {
                        PyObject *key = a->ma_table[i].me_key;
                        PyObject *aval, *bval;
-                       /* XXX What if PyObject_Compare raises an exception? */
-                       if (diff != NULL && PyObject_Compare(key, diff) > 0)
+                       if (diff != NULL) {
+                           cmp = PyObject_RichCompareBool(diff, key, Py_LT);
+                           if (cmp < 0)
+                               return NULL;
+                           if (cmp > 0)
                                continue;
+                       }
                        aval = a->ma_table[i].me_value;
                        bval = PyDict_GetItem((PyObject *)b, key);
-                       /* XXX What if PyObject_Compare raises an exception? */
-                       if (bval == NULL || PyObject_Compare(aval, bval) != 0)
+                       if (bval == NULL)
+                               cmp = 0;
+                       else {
+                           cmp = PyObject_RichCompareBool(aval, bval, Py_EQ);
+                           if (cmp < 0)
+                               return NULL;
+                       }
+                       if (cmp == 0)
                        {
                                diff = key;
                                *pval = aval;
@@ -960,12 +966,12 @@ dict_compare(dictobject *a, dictobject *b)
                return 1;       /* b is shorter */
        /* Same length -- check all keys */
        adiff = characterize(a, b, &aval);
-       if (PyErr_Occurred())
+       if (adiff == NULL && PyErr_Occurred())
                return -1;
        if (adiff == NULL)
                return 0;       /* a is a subset with the same length */
        bdiff = characterize(b, a, &bval);
-       if (PyErr_Occurred())
+       if (bdiff == NULL && PyErr_Occurred())
                return -1;
        /* bdiff == NULL would be impossible now */
        res = PyObject_Compare(adiff, bdiff);
@@ -974,86 +980,6 @@ dict_compare(dictobject *a, dictobject *b)
        return res;
 }
 
-#else /* !NEWCMP */
-
-static int
-dict_compare(dictobject *a, dictobject *b)
-{
-       PyObject *akeys, *bkeys;
-       int i, n, res;
-       if (a == b)
-               return 0;
-       if (a->ma_used == 0) {
-               if (b->ma_used != 0)
-                       return -1;
-               else
-                       return 0;
-       }
-       else {
-               if (b->ma_used == 0)
-                       return 1;
-       }
-       akeys = dict_keys(a, (PyObject *)NULL);
-       bkeys = dict_keys(b, (PyObject *)NULL);
-       if (akeys == NULL || bkeys == NULL) {
-               /* Oops, out of memory -- what to do? */
-               /* For now, sort on address! */
-               Py_XDECREF(akeys);
-               Py_XDECREF(bkeys);
-               if (a < b)
-                       return -1;
-               else
-                       return 1;
-       }
-       PyList_Sort(akeys);
-       PyList_Sort(bkeys);
-       n = a->ma_used < b->ma_used ? a->ma_used : b->ma_used; /* smallest */
-       res = 0;
-       for (i = 0; i < n; i++) {
-               PyObject *akey, *bkey, *aval, *bval;
-               long ahash, bhash;
-               akey = PyList_GetItem(akeys, i);
-               bkey = PyList_GetItem(bkeys, i);
-               res = PyObject_Compare(akey, bkey);
-               if (res != 0)
-                       break;
-#ifdef CACHE_HASH
-               if (!PyString_Check(akey) ||
-                   (ahash = ((PyStringObject *) akey)->ob_shash) == -1)
-#endif
-               {
-                       ahash = PyObject_Hash(akey);
-                       if (ahash == -1)
-                               PyErr_Clear(); /* Don't want errors here */
-               }
-#ifdef CACHE_HASH
-               if (!PyString_Check(bkey) ||
-                   (bhash = ((PyStringObject *) bkey)->ob_shash) == -1)
-#endif
-               {
-                       bhash = PyObject_Hash(bkey);
-                       if (bhash == -1)
-                               PyErr_Clear(); /* Don't want errors here */
-               }
-               aval = (a->ma_lookup)(a, akey, ahash) -> me_value;
-               bval = (b->ma_lookup)(b, bkey, bhash) -> me_value;
-               res = PyObject_Compare(aval, bval);
-               if (res != 0)
-                       break;
-       }
-       if (res == 0) {
-               if (a->ma_used < b->ma_used)
-                       res = -1;
-               else if (a->ma_used > b->ma_used)
-                       res = 1;
-       }
-       Py_DECREF(akeys);
-       Py_DECREF(bkeys);
-       return res;
-}
-
-#endif /* !NEWCMP */
-
 static PyObject *
 dict_has_key(register dictobject *mp, PyObject *args)
 {
@@ -1298,25 +1224,26 @@ PyTypeObject PyDict_Type = {
        "dictionary",
        sizeof(dictobject) + PyGC_HEAD_SIZE,
        0,
-       (destructor)dict_dealloc, /*tp_dealloc*/
-       (printfunc)dict_print, /*tp_print*/
-       (getattrfunc)dict_getattr, /*tp_getattr*/
-       0,                      /*tp_setattr*/
-       (cmpfunc)dict_compare, /*tp_compare*/
-       (reprfunc)dict_repr, /*tp_repr*/
-       0,                      /*tp_as_number*/
-       0,                      /*tp_as_sequence*/
-       &dict_as_mapping,       /*tp_as_mapping*/
-       0,              /* tp_hash */
-       0,              /* tp_call */
-       0,              /* tp_str */
-       0,              /* tp_getattro */
-       0,              /* tp_setattro */
-       0,              /* tp_as_buffer */
-       Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC, /*tp_flags*/
-       0,              /* tp_doc */
-       (traverseproc)dict_traverse,    /* tp_traverse */
-       (inquiry)dict_tp_clear,         /* tp_clear */
+       (destructor)dict_dealloc,               /* tp_dealloc */
+       (printfunc)dict_print,                  /* tp_print */
+       (getattrfunc)dict_getattr,              /* tp_getattr */
+       0,                                      /* tp_setattr */
+       (cmpfunc)dict_compare,                  /* tp_compare */
+       (reprfunc)dict_repr,                    /* tp_repr */
+       0,                                      /* tp_as_number */
+       0,                                      /* tp_as_sequence */
+       &dict_as_mapping,                       /* tp_as_mapping */
+       0,                                      /* tp_hash */
+       0,                                      /* tp_call */
+       0,                                      /* tp_str */
+       0,                                      /* tp_getattro */
+       0,                                      /* tp_setattro */
+       0,                                      /* tp_as_buffer */
+       Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC,     /* tp_flags */
+       0,                                      /* tp_doc */
+       (traverseproc)dict_traverse,            /* tp_traverse */
+       (inquiry)dict_tp_clear,                 /* tp_clear */
+       0,                                      /* tp_richcompare */
 };
 
 /* For backward compatibility with old dictionary interface */