Implement the intention of SF patch 472523 (but coded differently).

In the past, an object's tp_compare could return any value.  In 2.2
the docs were tightened to require it to return -1, 0 or 1; and -1 for
an error.

We now issue a warning if the value is not in this range.  When an
exception is raised, we allow -1 or -2 as return value, since -2 will
the recommended return value for errors in the future.  (Eventually
tp_compare will also be allowed to return +2, to indicate
NotImplemented; but that can only be implemented once we know all
extensions return a value in [-2...1].  Or perhaps it will require the
type to set a flag bit.)

I haven't decided yet whether to backport this to 2.2.x.  The patch
applies fine.  But is it fair to start warning in 2.2.2 about code
that worked flawlessly in 2.2.1?

diff --git a/Objects/object.c b/Objects/object.c
index b1bf2c3cdbdf1b8b3601986849a37f5a2a44bcb5..3207d8a556e551367d380df231a980c010bf87a6 100644 (file)
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -353,6 +353,46 @@ PyObject_Unicode(PyObject *v)
 #endif
 
 
+/* Helper to warn about deprecated tp_compare return values.  Return:
+   -2 for an exception;
+   -1 if v <  w;
+    0 if v == w;
+    1 if v  > w.
+   (This function cannot return 2.)
+*/
+static int
+adjust_tp_compare(int c)
+{
+       if (PyErr_Occurred()) {
+               if (c != -1 && c != -2) {
+                       PyObject *t, *v, *tb;
+                       PyErr_Fetch(&t, &v, &tb);
+                       if (PyErr_Warn(PyExc_RuntimeWarning,
+                                      "tp_compare didn't return -1 or -2 "
+                                      "for exception") < 0) {
+                               Py_XDECREF(t);
+                               Py_XDECREF(v);
+                               Py_XDECREF(tb);
+                       }
+                       else
+                               PyErr_Restore(t, v, tb);
+               }
+               return -2;
+       }
+       else if (c < -1 || c > 1) {
+               if (PyErr_Warn(PyExc_RuntimeWarning,
+                              "tp_compare didn't return -1, 0 or 1") < 0)
+                       return -2;
+               else
+                       return c < -1 ? -1 : 1;
+       }
+       else {
+               assert(c >= -1 && c <= 1);
+               return c;
+       }
+}
+
+
 /* Macro to get the tp_richcompare field of a type if defined */
 #define RICHCOMPARE(t) (PyType_HasFeature((t), Py_TPFLAGS_HAVE_RICHCOMPARE) \
                          ? (t)->tp_richcompare : NULL)
@@ -480,9 +520,7 @@ try_3way_compare(PyObject *v, PyObject *w)
        /* If both have the same (non-NULL) tp_compare, use it. */
        if (f != NULL && f == w->ob_type->tp_compare) {
                c = (*f)(v, w);
-               if (c < 0 && PyErr_Occurred())
-                       return -1;
-               return c < 0 ? -1 : c > 0 ? 1 : 0;
+               return adjust_tp_compare(c);
        }
 
        /* If either tp_compare is _PyObject_SlotCompare, that's safe. */
@@ -502,9 +540,7 @@ try_3way_compare(PyObject *v, PyObject *w)
                c = (*f)(v, w);
                Py_DECREF(v);
                Py_DECREF(w);
-               if (c < 0 && PyErr_Occurred())
-                       return -2;
-               return c < 0 ? -1 : c > 0 ? 1 : 0;
+               return adjust_tp_compare(c);
        }
 
        /* Try w's comparison, if defined */
@@ -512,9 +548,11 @@ try_3way_compare(PyObject *v, PyObject *w)
                c = (*f)(w, v); /* swapped! */
                Py_DECREF(v);
                Py_DECREF(w);
-               if (c < 0 && PyErr_Occurred())
-                       return -2;
-               return c < 0 ? 1 : c > 0 ? -1 : 0; /* negated! */
+               c = adjust_tp_compare(c);
+               if (c >= -1)
+                       return -c; /* Swapped! */
+               else
+                       return c;
        }
 
        /* No comparison defined */
@@ -591,11 +629,11 @@ default_3way_compare(PyObject *v, PyObject *w)
 #define CHECK_TYPES(o) PyType_HasFeature((o)->ob_type, Py_TPFLAGS_CHECKTYPES)
 
 /* Do a 3-way comparison, by hook or by crook.  Return:
-   -2 for an exception;
+   -2 for an exception (but see below);
    -1 if v <  w;
     0 if v == w;
     1 if v >  w;
-   If the object implements a tp_compare function, it returns
+   BUT: if the object implements a tp_compare function, it returns
    whatever this function returns (whether with an exception or not).
 */
 static int
@@ -607,9 +645,22 @@ do_cmp(PyObject *v, PyObject *w)
        if (v->ob_type == w->ob_type
            && (f = v->ob_type->tp_compare) != NULL) {
                c = (*f)(v, w);
-               if (c != 2 || !PyInstance_Check(v))
-                       return c;
-       }
+               if (PyInstance_Check(v)) {
+                       /* Instance tp_compare has a different signature.
+                          But if it returns undefined we fall through. */
+                       if (c != 2)
+                               return c;
+                       /* Else fall throug to try_rich_to_3way_compare() */
+               }
+               else
+                       return adjust_tp_compare(c);
+       }
+       /* We only get here if one of the following is true:
+          a) v and w have different types
+          b) v and w have the same type, which doesn't have tp_compare
+          c) v and w are instances, and either __cmp__ is not defined or
+             __cmp__ returns NotImplemented
+       */
        c = try_rich_to_3way_compare(v, w);
        if (c < 2)
                return c;
@@ -902,7 +953,8 @@ PyObject_RichCompare(PyObject *v, PyObject *w, int op)
                fcmp = v->ob_type->tp_compare;
                if (fcmp != NULL) {
                        int c = (*fcmp)(v, w);
-                       if (c < 0 && PyErr_Occurred()) {
+                       c = adjust_tp_compare(c);
+                       if (c == -2) {
                                res = NULL;
                                goto Done;
                        }