bpo-23699: Use a macro to reduce boilerplate code in rich comparison functions (GH-793)
diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst
index 0b4577f..3bdf45a 100644
--- a/Doc/c-api/typeobj.rst
+++ b/Doc/c-api/typeobj.rst
@@ -623,6 +623,22 @@
| :const:`Py_GE` | ``>=`` |
+----------------+------------+
+ The following macro is defined to ease writing rich comparison functions:
+
+ .. c:function:: PyObject *Py_RETURN_RICHCOMPARE(VAL_A, VAL_B, int op)
+
+ Return ``Py_True`` or ``Py_False`` from the function, depending on the
+ result of a comparison.
+ VAL_A and VAL_B must be orderable by C comparison operators (for example,
+ they may be C ints or floats). The third argument specifies the requested
+ operation, as for :c:func:`PyObject_RichCompare`.
+
+ The return value's reference count is properly incremented.
+
+ On error, sets an exception and returns NULL from the function.
+
+ .. versionadded:: 3.7
+
.. c:member:: Py_ssize_t PyTypeObject.tp_weaklistoffset
diff --git a/Include/object.h b/Include/object.h
index cb57359..b7051f3 100644
--- a/Include/object.h
+++ b/Include/object.h
@@ -931,6 +931,25 @@
#define Py_GT 4
#define Py_GE 5
+/*
+ * Macro for implementing rich comparisons
+ *
+ * Needs to be a macro because any C-comparable type can be used.
+ */
+#define Py_RETURN_RICHCOMPARE(val1, val2, op) \
+ do { \
+ switch (op) { \
+ case Py_EQ: if ((val1) == (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ case Py_NE: if ((val1) != (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ case Py_LT: if ((val1) < (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ case Py_GT: if ((val1) > (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ case Py_LE: if ((val1) <= (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ case Py_GE: if ((val1) >= (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
+ default: \
+ Py_UNREACHABLE(); \
+ } \
+ } while (0)
+
#ifndef Py_LIMITED_API
/* Maps Py_LT to Py_GT, ..., Py_GE to Py_LE.
* Defined in object.c.
diff --git a/Misc/ACKS b/Misc/ACKS
index ac7a258..1bd3c04 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1646,7 +1646,7 @@
Jaap Vermeulen
Nikita Vetoshkin
Al Vezza
-Petr Victorin
+Petr Viktorin
Jacques A. Vidrine
John Viega
Dino Viehland
diff --git a/Misc/NEWS.d/next/C API/2017-10-19-15-27-04.bpo-23699.-noVVc.rst b/Misc/NEWS.d/next/C API/2017-10-19-15-27-04.bpo-23699.-noVVc.rst
new file mode 100644
index 0000000..d873666
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2017-10-19-15-27-04.bpo-23699.-noVVc.rst
@@ -0,0 +1,2 @@
+Add Py_RETURN_RICHCOMPARE macro to reduce boilerplate code in rich
+comparison functions.
diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c
index 09b5579..b50cdda 100644
--- a/Modules/_datetimemodule.c
+++ b/Modules/_datetimemodule.c
@@ -1442,22 +1442,7 @@
static PyObject *
diff_to_bool(int diff, int op)
{
- PyObject *result;
- int istrue;
-
- switch (op) {
- case Py_EQ: istrue = diff == 0; break;
- case Py_NE: istrue = diff != 0; break;
- case Py_LE: istrue = diff <= 0; break;
- case Py_GE: istrue = diff >= 0; break;
- case Py_LT: istrue = diff < 0; break;
- case Py_GT: istrue = diff > 0; break;
- default:
- Py_UNREACHABLE();
- }
- result = istrue ? Py_True : Py_False;
- Py_INCREF(result);
- return result;
+ Py_RETURN_RICHCOMPARE(diff, 0, op);
}
/* Raises a "can't compare" TypeError and returns NULL. */
diff --git a/Modules/_tkinter.c b/Modules/_tkinter.c
index 005214e..7123da5 100644
--- a/Modules/_tkinter.c
+++ b/Modules/_tkinter.c
@@ -864,13 +864,10 @@
return repr;
}
-#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
-
static PyObject *
PyTclObject_richcompare(PyObject *self, PyObject *other, int op)
{
int result;
- PyObject *v;
/* neither argument should be NULL, unless something's gone wrong */
if (self == NULL || other == NULL) {
@@ -880,8 +877,7 @@
/* both arguments should be instances of PyTclObject */
if (!PyTclObject_Check(self) || !PyTclObject_Check(other)) {
- v = Py_NotImplemented;
- goto finished;
+ Py_RETURN_NOTIMPLEMENTED;
}
if (self == other)
@@ -890,33 +886,7 @@
else
result = strcmp(Tcl_GetString(((PyTclObject *)self)->value),
Tcl_GetString(((PyTclObject *)other)->value));
- /* Convert return value to a Boolean */
- 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;
- }
- finished:
- Py_INCREF(v);
- return v;
+ Py_RETURN_RICHCOMPARE(result, 0, op);
}
PyDoc_STRVAR(get_typename__doc__, "name of the Tcl type");
diff --git a/Modules/parsermodule.c b/Modules/parsermodule.c
index 929f2de..2b98be4 100644
--- a/Modules/parsermodule.c
+++ b/Modules/parsermodule.c
@@ -299,13 +299,10 @@
*
*/
-#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
-
static PyObject *
parser_richcompare(PyObject *left, PyObject *right, int op)
{
int result;
- PyObject *v;
/* neither argument should be NULL, unless something's gone wrong */
if (left == NULL || right == NULL) {
@@ -315,8 +312,7 @@
/* both arguments should be instances of PyST_Object */
if (!PyST_Object_Check(left) || !PyST_Object_Check(right)) {
- v = Py_NotImplemented;
- goto finished;
+ Py_RETURN_NOTIMPLEMENTED;
}
if (left == right)
@@ -326,33 +322,7 @@
result = parser_compare_nodes(((PyST_Object *)left)->st_node,
((PyST_Object *)right)->st_node);
- /* Convert return value to a Boolean */
- 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;
- }
- finished:
- Py_INCREF(v);
- return v;
+ Py_RETURN_RICHCOMPARE(result, 0, op);
}
/* parser_newstobject(node* st)
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
index 1cde6e8..f2f5cc8 100644
--- a/Modules/selectmodule.c
+++ b/Modules/selectmodule.c
@@ -1929,27 +1929,7 @@
: 0;
#undef CMP
- switch (op) {
- case Py_EQ:
- result = (result == 0);
- break;
- case Py_NE:
- result = (result != 0);
- break;
- case Py_LE:
- result = (result <= 0);
- break;
- case Py_GE:
- result = (result >= 0);
- break;
- case Py_LT:
- result = (result < 0);
- break;
- case Py_GT:
- result = (result > 0);
- break;
- }
- return PyBool_FromLong((long)result);
+ Py_RETURN_RICHCOMPARE(result, 0, op);
}
static PyTypeObject kqueue_event_Type = {
diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c
index c92cfc0..83c3549 100644
--- a/Objects/bytearrayobject.c
+++ b/Objects/bytearrayobject.c
@@ -1012,8 +1012,6 @@
{
Py_ssize_t self_size, other_size;
Py_buffer self_bytes, other_bytes;
- PyObject *res;
- Py_ssize_t minsize;
int cmp, rc;
/* Bytes can be compared to anything that supports the (binary)
@@ -1049,38 +1047,25 @@
if (self_size != other_size && (op == Py_EQ || op == Py_NE)) {
/* Shortcut: if the lengths differ, the objects differ */
- cmp = (op == Py_NE);
+ PyBuffer_Release(&self_bytes);
+ PyBuffer_Release(&other_bytes);
+ return PyBool_FromLong((op == Py_NE));
}
else {
- minsize = self_size;
- if (other_size < minsize)
- minsize = other_size;
-
- cmp = memcmp(self_bytes.buf, other_bytes.buf, minsize);
+ cmp = memcmp(self_bytes.buf, other_bytes.buf,
+ Py_MIN(self_size, other_size));
/* In ISO C, memcmp() guarantees to use unsigned bytes! */
- if (cmp == 0) {
- if (self_size < other_size)
- cmp = -1;
- else if (self_size > other_size)
- cmp = 1;
+ PyBuffer_Release(&self_bytes);
+ PyBuffer_Release(&other_bytes);
+
+ if (cmp != 0) {
+ Py_RETURN_RICHCOMPARE(cmp, 0, op);
}
- switch (op) {
- case Py_LT: cmp = cmp < 0; break;
- case Py_LE: cmp = cmp <= 0; break;
- case Py_EQ: cmp = cmp == 0; break;
- case Py_NE: cmp = cmp != 0; break;
- case Py_GT: cmp = cmp > 0; break;
- case Py_GE: cmp = cmp >= 0; break;
- }
+ Py_RETURN_RICHCOMPARE(self_size, other_size, op);
}
- res = cmp ? Py_True : Py_False;
- PyBuffer_Release(&self_bytes);
- PyBuffer_Release(&other_bytes);
- Py_INCREF(res);
- return res;
}
static void
diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
index 7ba90aa..a921d9c 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -1566,7 +1566,6 @@
int c;
Py_ssize_t len_a, len_b;
Py_ssize_t min_len;
- PyObject *result;
int rc;
/* Make sure both arguments are strings. */
@@ -1599,7 +1598,7 @@
}
}
}
- result = Py_NotImplemented;
+ Py_RETURN_NOTIMPLEMENTED;
}
else if (a == b) {
switch (op) {
@@ -1607,12 +1606,12 @@
case Py_LE:
case Py_GE:
/* a string is equal to itself */
- result = Py_True;
+ Py_RETURN_TRUE;
break;
case Py_NE:
case Py_LT:
case Py_GT:
- result = Py_False;
+ Py_RETURN_FALSE;
break;
default:
PyErr_BadArgument();
@@ -1622,7 +1621,7 @@
else if (op == Py_EQ || op == Py_NE) {
int eq = bytes_compare_eq(a, b);
eq ^= (op == Py_NE);
- result = eq ? Py_True : Py_False;
+ return PyBool_FromLong(eq);
}
else {
len_a = Py_SIZE(a);
@@ -1635,22 +1634,10 @@
}
else
c = 0;
- if (c == 0)
- c = (len_a < len_b) ? -1 : (len_a > len_b) ? 1 : 0;
- switch (op) {
- case Py_LT: c = c < 0; break;
- case Py_LE: c = c <= 0; break;
- case Py_GT: c = c > 0; break;
- case Py_GE: c = c >= 0; break;
- default:
- PyErr_BadArgument();
- return NULL;
- }
- result = c ? Py_True : Py_False;
+ if (c != 0)
+ Py_RETURN_RICHCOMPARE(c, 0, op);
+ Py_RETURN_RICHCOMPARE(len_a, len_b, op);
}
-
- Py_INCREF(result);
- return result;
}
static Py_hash_t
diff --git a/Objects/cellobject.c b/Objects/cellobject.c
index af19229..7b05e61 100644
--- a/Objects/cellobject.c
+++ b/Objects/cellobject.c
@@ -53,22 +53,15 @@
PyObject_GC_Del(op);
}
-#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
-
static PyObject *
cell_richcompare(PyObject *a, PyObject *b, int op)
{
- 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;
+ Py_RETURN_NOTIMPLEMENTED;
}
/* compare cells by contents; empty cells come before anything else */
@@ -77,32 +70,7 @@
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;
+ Py_RETURN_RICHCOMPARE(b == NULL, a == NULL, op);
}
static PyObject *
diff --git a/Objects/descrobject.c b/Objects/descrobject.c
index 5dc27ef..71d5224 100644
--- a/Objects/descrobject.c
+++ b/Objects/descrobject.c
@@ -1035,22 +1035,16 @@
Py_TRASHCAN_SAFE_END(wp)
}
-#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
-
static PyObject *
wrapper_richcompare(PyObject *a, PyObject *b, int op)
{
- intptr_t 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;
+ Py_RETURN_NOTIMPLEMENTED;
}
/* compare by descriptor address; if the descriptors are the same,
@@ -1063,32 +1057,7 @@
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;
+ Py_RETURN_RICHCOMPARE(a_descr, b_descr, op);
}
static Py_hash_t
diff --git a/Objects/listobject.c b/Objects/listobject.c
index 8585322..7eba61e 100644
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -2330,13 +2330,10 @@
if (Py_SIZE(vl) != Py_SIZE(wl) && (op == Py_EQ || op == Py_NE)) {
/* Shortcut: if the lengths differ, the lists differ */
- PyObject *res;
if (op == Py_EQ)
- res = Py_False;
+ Py_RETURN_FALSE;
else
- res = Py_True;
- Py_INCREF(res);
- return res;
+ Py_RETURN_TRUE;
}
/* Search for the first index where items are different */
@@ -2351,25 +2348,7 @@
if (i >= Py_SIZE(vl) || i >= Py_SIZE(wl)) {
/* No more items to compare -- compare sizes */
- Py_ssize_t vs = Py_SIZE(vl);
- Py_ssize_t ws = Py_SIZE(wl);
- int cmp;
- PyObject *res;
- switch (op) {
- case Py_LT: cmp = vs < ws; break;
- case Py_LE: cmp = vs <= ws; break;
- case Py_EQ: cmp = vs == ws; break;
- case Py_NE: cmp = vs != ws; break;
- case Py_GT: cmp = vs > ws; break;
- case Py_GE: cmp = vs >= ws; break;
- default: return NULL; /* cannot happen */
- }
- if (cmp)
- res = Py_True;
- else
- res = Py_False;
- Py_INCREF(res);
- return res;
+ Py_RETURN_RICHCOMPARE(Py_SIZE(vl), Py_SIZE(wl), op);
}
/* We have an item that differs -- shortcuts for EQ/NE */
diff --git a/Objects/longobject.c b/Objects/longobject.c
index c71b783..6fd4feb 100644
--- a/Objects/longobject.c
+++ b/Objects/longobject.c
@@ -2915,45 +2915,16 @@
return sign < 0 ? -1 : sign > 0 ? 1 : 0;
}
-#define TEST_COND(cond) \
- ((cond) ? Py_True : Py_False)
-
static PyObject *
long_richcompare(PyObject *self, PyObject *other, int op)
{
int result;
- PyObject *v;
CHECK_BINOP(self, other);
if (self == other)
result = 0;
else
result = long_compare((PyLongObject*)self, (PyLongObject*)other);
- /* Convert the return value to a Boolean */
- 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 == -1);
- break;
- case Py_GT:
- v = TEST_COND(result == 1);
- break;
- default:
- PyErr_BadArgument();
- return NULL;
- }
- Py_INCREF(v);
- return v;
+ Py_RETURN_RICHCOMPARE(result, 0, op);
}
static Py_hash_t
diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c
index c150af8..35decd8 100644
--- a/Objects/tupleobject.c
+++ b/Objects/tupleobject.c
@@ -647,23 +647,7 @@
if (i >= vlen || i >= wlen) {
/* No more items to compare -- compare sizes */
- int cmp;
- PyObject *res;
- switch (op) {
- case Py_LT: cmp = vlen < wlen; break;
- case Py_LE: cmp = vlen <= wlen; break;
- case Py_EQ: cmp = vlen == wlen; break;
- case Py_NE: cmp = vlen != wlen; break;
- case Py_GT: cmp = vlen > wlen; break;
- case Py_GE: cmp = vlen >= wlen; break;
- default: return NULL; /* cannot happen */
- }
- if (cmp)
- res = Py_True;
- else
- res = Py_False;
- Py_INCREF(res);
- return res;
+ Py_RETURN_RICHCOMPARE(vlen, wlen, op);
}
/* We have an item that differs -- shortcuts for EQ/NE */
diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
index 6533f41..194c5bc 100644
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -11152,14 +11152,10 @@
return unicode_compare_eq(left, right_uni);
}
-#define TEST_COND(cond) \
- ((cond) ? Py_True : Py_False)
-
PyObject *
PyUnicode_RichCompare(PyObject *left, PyObject *right, int op)
{
int result;
- PyObject *v;
if (!PyUnicode_Check(left) || !PyUnicode_Check(right))
Py_RETURN_NOTIMPLEMENTED;
@@ -11174,13 +11170,11 @@
case Py_LE:
case Py_GE:
/* a string is equal to itself */
- v = Py_True;
- break;
+ Py_RETURN_TRUE;
case Py_NE:
case Py_LT:
case Py_GT:
- v = Py_False;
- break;
+ Py_RETURN_FALSE;
default:
PyErr_BadArgument();
return NULL;
@@ -11189,32 +11183,12 @@
else if (op == Py_EQ || op == Py_NE) {
result = unicode_compare_eq(left, right);
result ^= (op == Py_NE);
- v = TEST_COND(result);
+ return PyBool_FromLong(result);
}
else {
result = unicode_compare(left, right);
-
- /* Convert the return value to a Boolean */
- switch (op) {
- 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 == -1);
- break;
- case Py_GT:
- v = TEST_COND(result == 1);
- break;
- default:
- PyErr_BadArgument();
- return NULL;
- }
+ Py_RETURN_RICHCOMPARE(result, 0, op);
}
- Py_INCREF(v);
- return v;
}
int