Skip to content

bpo-23699: Use a macro to reduce boilerplate code in rich comparison functions #793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ncoghlan merged 1 commit into from
Nov 2, 2017
Merged

bpo-23699: Use a macro to reduce boilerplate code in rich comparison functions #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Doc/c-api/typeobj.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,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.7


.. c:member:: Py_ssize_t PyTypeObject.tp_weaklistoffset

Expand Down
19 changes: 19 additions & 0 deletions Include/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,25 @@ 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; \
default: \
Py_UNREACHABLE(); \
} \
} while (0)

#ifndef Py_LIMITED_API
/* Maps Py_LT to Py_GT, ..., Py_GE to Py_LE.
* Defined in object.c.
Expand Down
2 changes: 1 addition & 1 deletion Misc/ACKS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,7 @@ Mike Verdone
Jaap Vermeulen
Nikita Vetoshkin
Al Vezza
Petr Victorin
Petr Viktorin
Jacques A. Vidrine
John Viega
Dino Viehland
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/C API/2017-10-19-15-27-04.bpo-23699.-noVVc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add Py_RETURN_RICHCOMPARE macro to reduce boilerplate code in rich
comparison functions.
17 changes: 1 addition & 16 deletions Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,22 +1442,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:
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. */
Expand Down
34 changes: 2 additions & 32 deletions Modules/_tkinter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,13 +864,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) {
Expand All @@ -880,8 +877,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)
Expand All @@ -890,33 +886,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");
Expand Down
34 changes: 2 additions & 32 deletions Modules/parsermodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,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) {
Expand All @@ -315,8 +312,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)
Expand All @@ -326,33 +322,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)
Expand Down
22 changes: 1 addition & 21 deletions Modules/selectmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1905,27 +1905,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 = {
Expand Down
37 changes: 11 additions & 26 deletions Objects/bytearrayobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,8 +991,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, rc;

/* Bytes can be compared to anything that supports the (binary)
Expand Down Expand Up @@ -1028,38 +1026,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
Expand Down
27 changes: 7 additions & 20 deletions Objects/bytesobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1566,7 +1566,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;
int rc;

/* Make sure both arguments are strings. */
Expand Down Expand Up @@ -1599,20 +1598,20 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
}
}
}
result = Py_NotImplemented;
Py_RETURN_NOTIMPLEMENTED;
}
else if (a == b) {
switch (op) {
case Py_EQ:
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();
Expand All @@ -1622,7 +1621,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);
Expand All @@ -1635,22 +1634,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
Expand Down
Loading