Issue #1717, stage 2: remove uses of tp_compare in Modules and most

Objects.

4 files changed, 119 insertions, 41 deletions

diff --git a/Objects/cellobject.c b/Objects/cellobject.c
index 50a3897..c2194b7 100644
--- a/Objects/cellobject.c
+++ b/Objects/cellobject.c
@@ -51,16 +51,56 @@ cell_dealloc(PyCellObject *op)
 	PyObject_GC_Del(op);
 }
 
-static int
-cell_compare(PyCellObject *a, PyCellObject *b)
+#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
+
+static PyObject *
+cell_richcompare(PyObject *a, PyObject *b, int op)
 {
-	if (a->ob_ref == NULL) {
-		if (b->ob_ref == NULL)
-			return 0;
-		return -1;
-	} else if (b->ob_ref == NULL)
-		return 1;
-	return PyObject_Compare(a->ob_ref, b->ob_ref);
+	int result;
+	PyObject *v;
+
+	/* neither argument should be NULL, unless something's gone wrong */
+	assert(a != NULL && b != NULL);
+
+	/* both arguments should be instances of PyCellObject */
+	if (!PyCell_Check(a) || !PyCell_Check(b)) {
+		v = Py_NotImplemented;
+		Py_INCREF(v);
+		return v;
+	}
+
+	/* compare cells by contents; empty cells come before anything else */
+	a = ((PyCellObject *)a)->ob_ref;
+	b = ((PyCellObject *)b)->ob_ref;
+	if (a != NULL && b != NULL)
+		return PyObject_RichCompare(a, b, op);
+
+	result = (b == NULL) - (a == NULL);
+	switch (op) {
+	case Py_EQ:
+		v = TEST_COND(result == 0);
+		break;
+	case Py_NE:
+		v = TEST_COND(result != 0);
+		break;
+	case Py_LE:
+		v = TEST_COND(result <= 0);
+		break;
+	case Py_GE:
+		v = TEST_COND(result >= 0);
+		break;
+	case Py_LT:
+		v = TEST_COND(result < 0);
+		break;
+	case Py_GT:
+		v = TEST_COND(result > 0);
+		break;
+	default:
+		PyErr_BadArgument();
+		return NULL;
+	}
+	Py_INCREF(v);
+	return v;
 }
 
 static PyObject *
@@ -114,7 +154,7 @@ PyTypeObject PyCell_Type = {
 	0,                                      /* tp_print */
 	0,	                                /* tp_getattr */
 	0,					/* tp_setattr */
-	(cmpfunc)cell_compare,					/* tp_compare */
+	0,					/* tp_compare */
 	(reprfunc)cell_repr,			/* tp_repr */
 	0,					/* tp_as_number */
 	0,			                /* tp_as_sequence */
@@ -129,7 +169,7 @@ PyTypeObject PyCell_Type = {
  	0,					/* tp_doc */
  	(traverseproc)cell_traverse,		/* tp_traverse */
  	(inquiry)cell_clear,			/* tp_clear */
-	0,					/* tp_richcompare */
+	cell_richcompare,			/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
 	0, 					/* tp_iter */
 	0,					/* tp_iternext */
diff --git a/Objects/descrobject.c b/Objects/descrobject.c
index 9f4c2a6..a623925 100644
--- a/Objects/descrobject.c
+++ b/Objects/descrobject.c
@@ -774,12 +774,6 @@ proxy_traverse(PyObject *self, visitproc visit, void *arg)
 	return 0;
 }
 
-static int
-proxy_compare(proxyobject *v, PyObject *w)
-{
-	return PyObject_Compare(v->dict, w);
-}
-
 static PyObject *
 proxy_richcompare(proxyobject *v, PyObject *w, int op)
 {
@@ -796,7 +790,7 @@ PyTypeObject PyDictProxy_Type = {
 	0,					/* tp_print */
 	0,					/* tp_getattr */
 	0,					/* tp_setattr */
-	(cmpfunc)proxy_compare,			/* tp_compare */
+	0,					/* tp_compare */
 	0,					/* tp_repr */
 	0,					/* tp_as_number */
 	&proxy_as_sequence,			/* tp_as_sequence */
@@ -844,12 +838,17 @@ PyDictProxy_New(PyObject *dict)
 /* This has no reason to be in this file except that adding new files is a
    bit of a pain */
 
+/* forward */
+static PyTypeObject wrappertype;
+
 typedef struct {
 	PyObject_HEAD
 	PyWrapperDescrObject *descr;
 	PyObject *self;
 } wrapperobject;
 
+#define Wrapper_Check(v) (Py_TYPE(v) == &wrappertype)
+
 static void
 wrapper_dealloc(wrapperobject *wp)
 {
@@ -861,13 +860,60 @@ wrapper_dealloc(wrapperobject *wp)
 	Py_TRASHCAN_SAFE_END(wp)
 }
 
-static int
-wrapper_compare(wrapperobject *a, wrapperobject *b)
+#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
+
+static PyObject *
+wrapper_richcompare(PyObject *a, PyObject *b, int op)
 {
-	if (a->descr == b->descr)
-		return PyObject_Compare(a->self, b->self);
-	else
-		return (a->descr < b->descr) ? -1 : 1;
+	int result;
+	PyObject *v;
+	PyWrapperDescrObject *a_descr, *b_descr;
+
+	assert(a != NULL && b != NULL);
+
+	/* both arguments should be wrapperobjects */
+	if (!Wrapper_Check(a) || !Wrapper_Check(b)) {
+		v = Py_NotImplemented;
+		Py_INCREF(v);
+		return v;
+	}
+
+	/* compare by descriptor address; if the descriptors are the same,
+	   compare by the objects they're bound to */
+	a_descr = ((wrapperobject *)a)->descr;
+	b_descr = ((wrapperobject *)b)->descr;
+	if (a_descr == b_descr) {
+		a = ((wrapperobject *)a)->self;
+		b = ((wrapperobject *)b)->self;
+		return PyObject_RichCompare(a, b, op);
+	}
+
+	result = a_descr - b_descr;
+	switch (op) {
+	case Py_EQ:
+		v = TEST_COND(result == 0);
+		break;
+	case Py_NE:
+		v = TEST_COND(result != 0);
+		break;
+	case Py_LE:
+		v = TEST_COND(result <= 0);
+		break;
+	case Py_GE:
+		v = TEST_COND(result >= 0);
+		break;
+	case Py_LT:
+		v = TEST_COND(result < 0);
+		break;
+	case Py_GT:
+		v = TEST_COND(result > 0);
+		break;
+	default:
+		PyErr_BadArgument();
+		return NULL;
+	}
+	Py_INCREF(v);
+	return v;
 }
 
 static long
@@ -977,7 +1023,7 @@ static PyTypeObject wrappertype = {
 	0,					/* tp_print */
 	0,					/* tp_getattr */
 	0,					/* tp_setattr */
-	(cmpfunc)wrapper_compare,		/* tp_compare */
+	0,					/* tp_compare */
 	(reprfunc)wrapper_repr,			/* tp_repr */
 	0,					/* tp_as_number */
 	0,					/* tp_as_sequence */
@@ -992,7 +1038,7 @@ static PyTypeObject wrappertype = {
  	0,					/* tp_doc */
 	wrapper_traverse,			/* tp_traverse */
  	0,					/* tp_clear */
-	0,					/* tp_richcompare */
+	wrapper_richcompare,			/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
 	0,					/* tp_iter */
 	0,					/* tp_iternext */
diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c
index f9a9cc9..68f06a4 100644
--- a/Objects/rangeobject.c
+++ b/Objects/rangeobject.c
@@ -123,7 +123,7 @@ range_length_obj(rangeobject *r)
     Algorithm is equal to that of get_len_of_range(), but it operates
     on PyObjects (which are assumed to be PyLong or PyInt objects).
     ---------------------------------------------------------------*/
-    int cmp_result, cmp_call;
+    int cmp_result;
     PyObject *lo, *hi;
     PyObject *step = NULL;
     PyObject *diff = NULL;
@@ -134,13 +134,12 @@ range_length_obj(rangeobject *r)
     PyObject *zero = PyLong_FromLong(0);
     if (zero == NULL)
         return NULL;
-    cmp_call = PyObject_Cmp(r->step, zero, &cmp_result);
+    cmp_result = PyObject_RichCompareBool(r->step, zero, Py_GT);
     Py_DECREF(zero);
-    if (cmp_call == -1)
+    if (cmp_result == -1)
         return NULL;
 
-    assert(cmp_result != 0);
-    if (cmp_result > 0) {
+    if (cmp_result == 1) {
         lo = r->start;
         hi = r->stop;
         step = r->step;
@@ -154,7 +153,7 @@ range_length_obj(rangeobject *r)
     }
 
     /* if (lo >= hi), return length of 0. */
-    if (PyObject_Compare(lo, hi) >= 0) {
+    if (PyObject_RichCompareBool(lo, hi, Py_GE) == 1) {
         Py_XDECREF(step);
         return PyLong_FromLong(0);
     }
diff --git a/Objects/setobject.c b/Objects/setobject.c
index d5d96ca..3dc396c 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -1824,13 +1824,6 @@ set_richcompare(PySetObject *v, PyObject *w, int op)
 	return Py_NotImplemented;
 }
 
-static int
-set_nocmp(PyObject *self, PyObject *other)
-{
-	PyErr_SetString(PyExc_TypeError, "cannot compare sets using cmp()");
-	return -1;
-}
-
 static PyObject *
 set_add(PySetObject *so, PyObject *key)
 {
@@ -2111,7 +2104,7 @@ PyTypeObject PySet_Type = {
 	0,				/* tp_print */
 	0,				/* tp_getattr */
 	0,				/* tp_setattr */
-	set_nocmp,			/* tp_compare */
+	0,				/* tp_compare */
 	(reprfunc)set_repr,		/* tp_repr */
 	&set_as_number,			/* tp_as_number */
 	&set_as_sequence,		/* tp_as_sequence */
@@ -2208,7 +2201,7 @@ PyTypeObject PyFrozenSet_Type = {
 	0,				/* tp_print */
 	0,				/* tp_getattr */
 	0,				/* tp_setattr */
-	set_nocmp,			/* tp_compare */
+	0,				/* tp_compare */
 	(reprfunc)set_repr,		/* tp_repr */
 	&frozenset_as_number,		/* tp_as_number */
 	&set_as_sequence,		/* tp_as_sequence */