From ebd5a81dff1fed14fb2e5eafe1fa4d11c18ba115 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 18 Mar 2015 14:14:38 +0100 Subject: [PATCH] Use a macro to reduce boilerplate in rich comparison functions --- Doc/c-api/typeobj.rst | 16 ++++++++++++++++ Include/object.h | 18 ++++++++++++++++++ Modules/_datetimemodule.c | 18 +----------------- Modules/_tkinter.c | 34 ++-------------------------------- Modules/arraymodule.c | 19 +------------------ Modules/parsermodule.c | 34 ++-------------------------------- Modules/selectmodule.c | 22 +--------------------- Objects/bytearrayobject.c | 37 +++++++++++-------------------------- Objects/bytesobject.c | 27 +++++++-------------------- Objects/cellobject.c | 36 ++---------------------------------- Objects/descrobject.c | 35 ++--------------------------------- Objects/listobject.c | 33 +++++---------------------------- Objects/longobject.c | 31 +------------------------------ Objects/tupleobject.c | 24 +++--------------------- Objects/unicodeobject.c | 35 ++++------------------------------- 15 files changed, 76 insertions(+), 343 deletions(-) diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index ee33a67..d31b237 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -620,6 +620,22 @@ type objects) *must* have the :attr:`ob_size` field. | :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.5 + .. c:member:: long PyTypeObject.tp_weaklistoffset diff --git a/Include/object.h b/Include/object.h index e173438..252047f 100644 --- a/Include/object.h +++ b/Include/object.h @@ -889,6 +889,24 @@ PyAPI_DATA(PyObject) _Py_NotImplementedStruct; /* Don't use this directly */ #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; \ + } \ + PyErr_BadArgument(); return NULL; \ + } while (0) + /* Maps Py_LT to Py_GT, ..., Py_GE to Py_LE. * Defined in object.c. */ diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 7e4be5b..8e32c01 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -1365,23 +1365,7 @@ build_struct_time(int y, int m, int d, int hh, int mm, int ss, int dstflag) 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: - assert(! "op unknown"); - istrue = 0; /* To shut up compiler */ - } - 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 4da836e..b4d7d9a 100644 --- a/Modules/_tkinter.c +++ b/Modules/_tkinter.c @@ -801,13 +801,10 @@ PyTclObject_repr(PyTclObject *self) 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) { @@ -817,8 +814,7 @@ PyTclObject_richcompare(PyObject *self, PyObject *other, int op) /* 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) @@ -827,33 +823,7 @@ PyTclObject_richcompare(PyObject *self, PyObject *other, int op) 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/arraymodule.c b/Modules/arraymodule.c index 8d0462d..dcf0391 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -674,24 +674,7 @@ array_richcompare(PyObject *v, PyObject *w, int op) if (k) { /* No more items to compare -- compare sizes */ - Py_ssize_t vs = Py_SIZE(va); - Py_ssize_t ws = Py_SIZE(wa); - int cmp; - 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(va), Py_SIZE(wa), op); } /* We have an item that differs. First, shortcuts for EQ/NE */ diff --git a/Modules/parsermodule.c b/Modules/parsermodule.c index 2a16dba..0613212 100644 --- a/Modules/parsermodule.c +++ b/Modules/parsermodule.c @@ -300,13 +300,10 @@ parser_compare_nodes(node *left, node *right) * */ -#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) { @@ -316,8 +313,7 @@ parser_richcompare(PyObject *left, PyObject *right, int op) /* 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) @@ -327,33 +323,7 @@ parser_richcompare(PyObject *left, PyObject *right, int op) 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 590aeca..2eed6c7 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -1895,27 +1895,7 @@ kqueue_event_richcompare(kqueue_event_Object *s, kqueue_event_Object *o, result = 0; } - 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 b9477ca..14e519c 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -1009,8 +1009,6 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op) { Py_ssize_t self_size, other_size; Py_buffer self_bytes, other_bytes; - PyObject *res; - Py_ssize_t minsize; int cmp; /* Bytes can be compared to anything that supports the (binary) @@ -1042,38 +1040,25 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op) 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); - 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; + if (cmp != 0) { + Py_RETURN_RICHCOMPARE(cmp, 0, op); } + + 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 d2b52c7..92dc830 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -1418,7 +1418,6 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) int c; Py_ssize_t len_a, len_b; Py_ssize_t min_len; - PyObject *result; /* Make sure both arguments are strings. */ if (!(PyBytes_Check(a) && PyBytes_Check(b))) { @@ -1440,7 +1439,7 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) return NULL; } } - result = Py_NotImplemented; + Py_RETURN_NOTIMPLEMENTED; } else if (a == b) { switch (op) { @@ -1448,12 +1447,12 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) 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(); @@ -1463,7 +1462,7 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) 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); @@ -1476,22 +1475,10 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op) } 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 eb5ad98..8ac7278 100644 --- a/Objects/cellobject.c +++ b/Objects/cellobject.c @@ -51,22 +51,15 @@ cell_dealloc(PyCellObject *op) 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 */ @@ -75,32 +68,7 @@ cell_richcompare(PyObject *a, PyObject *b, int op) 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 24b24e7..9ccd865 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1028,22 +1028,16 @@ wrapper_dealloc(wrapperobject *wp) Py_TRASHCAN_SAFE_END(wp) } -#define TEST_COND(cond) ((cond) ? Py_True : Py_False) - static PyObject * wrapper_richcompare(PyObject *a, PyObject *b, int op) { - Py_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, @@ -1056,32 +1050,7 @@ wrapper_richcompare(PyObject *a, PyObject *b, int op) 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 45e54ce..2f596f6 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -2235,13 +2235,10 @@ list_richcompare(PyObject *v, PyObject *w, int op) 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 */ @@ -2256,35 +2253,15 @@ list_richcompare(PyObject *v, PyObject *w, int op) 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 */ if (op == Py_EQ) { - Py_INCREF(Py_False); - return Py_False; + Py_RETURN_FALSE; } if (op == Py_NE) { - Py_INCREF(Py_True); - return Py_True; + Py_RETURN_TRUE; } /* Compare the final item again using the proper operator */ diff --git a/Objects/longobject.c b/Objects/longobject.c index 40da0b1..fcf1f8f 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -2745,45 +2745,16 @@ long_compare(PyLongObject *a, PyLongObject *b) 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 7efa1a6..d7629be 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -603,33 +603,15 @@ tuplerichcompare(PyObject *v, PyObject *w, int op) 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 */ if (op == Py_EQ) { - Py_INCREF(Py_False); - return Py_False; + Py_RETURN_FALSE; } if (op == Py_NE) { - Py_INCREF(Py_True); - return Py_True; + Py_RETURN_TRUE; } /* Compare the final item again using the proper operator */ diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 84e67e6..9f5a9af 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -10820,15 +10820,10 @@ PyUnicode_CompareWithASCIIString(PyObject* uni, const char* str) } } - -#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; @@ -10843,13 +10838,11 @@ PyUnicode_RichCompare(PyObject *left, PyObject *right, int op) 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; @@ -10858,32 +10851,12 @@ PyUnicode_RichCompare(PyObject *left, PyObject *right, int op) 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 -- 2.1.0