Skip to content

Commit e8b1965

Browse files
stratakisncoghlan
stratakis
authored and
ncoghlan
committed
bpo-23699: Use a macro to reduce boilerplate code in rich comparison functions (pythonGH-793)
1 parent 4f469c0 commit e8b1965

16 files changed

Lines changed: 75 additions & 316 deletions

‎Doc/c-api/typeobj.rst‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,22 @@ type objects) *must* have the :attr:`ob_size` field.
623623
| :const:`Py_GE` | ``>=`` |
624624
+----------------+------------+
625625

626+
The following macro is defined to ease writing rich comparison functions:
627+
628+
.. c:function:: PyObject *Py_RETURN_RICHCOMPARE(VAL_A, VAL_B, int op)
629+
630+
Return ``Py_True`` or ``Py_False`` from the function, depending on the
631+
result of a comparison.
632+
VAL_A and VAL_B must be orderable by C comparison operators (for example,
633+
they may be C ints or floats). The third argument specifies the requested
634+
operation, as for :c:func:`PyObject_RichCompare`.
635+
636+
The return value's reference count is properly incremented.
637+
638+
On error, sets an exception and returns NULL from the function.
639+
640+
.. versionadded:: 3.7
641+
626642
627643
.. c:member:: Py_ssize_t PyTypeObject.tp_weaklistoffset
628644

‎Include/object.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,25 @@ PyAPI_DATA(PyObject) _Py_NotImplementedStruct; /* Don't use this directly */
931931
#define Py_GT 4
932932
#define Py_GE 5
933933

934+
/*
935+
* Macro for implementing rich comparisons
936+
*
937+
* Needs to be a macro because any C-comparable type can be used.
938+
*/
939+
#define Py_RETURN_RICHCOMPARE(val1, val2, op) \
940+
do { \
941+
switch (op) { \
942+
case Py_EQ: if ((val1) == (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
943+
case Py_NE: if ((val1) != (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
944+
case Py_LT: if ((val1) < (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
945+
case Py_GT: if ((val1) > (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
946+
case Py_LE: if ((val1) <= (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
947+
case Py_GE: if ((val1) >= (val2)) Py_RETURN_TRUE; Py_RETURN_FALSE; \
948+
default: \
949+
Py_UNREACHABLE(); \
950+
} \
951+
} while (0)
952+
934953
#ifndef Py_LIMITED_API
935954
/* Maps Py_LT to Py_GT, ..., Py_GE to Py_LE.
936955
* Defined in object.c.

‎Misc/ACKS‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1646,7 +1646,7 @@ Mike Verdone
16461646
Jaap Vermeulen
16471647
Nikita Vetoshkin
16481648
Al Vezza
1649-
Petr Victorin
1649+
Petr Viktorin
16501650
Jacques A. Vidrine
16511651
John Viega
16521652
Dino Viehland

‎Misc/NEWS.d/next/C API/2017-10-19-15-27-04.bpo-23699.-noVVc.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add Py_RETURN_RICHCOMPARE macro to reduce boilerplate code in rich
2+
comparison functions.

‎Modules/_datetimemodule.c‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,22 +1442,7 @@ build_struct_time(int y, int m, int d, int hh, int mm, int ss, int dstflag)
14421442
static PyObject *
14431443
diff_to_bool(int diff, int op)
14441444
{
1445-
PyObject *result;
1446-
int istrue;
1447-
1448-
switch (op) {
1449-
case Py_EQ: istrue = diff == 0; break;
1450-
case Py_NE: istrue = diff != 0; break;
1451-
case Py_LE: istrue = diff <= 0; break;
1452-
case Py_GE: istrue = diff >= 0; break;
1453-
case Py_LT: istrue = diff < 0; break;
1454-
case Py_GT: istrue = diff > 0; break;
1455-
default:
1456-
Py_UNREACHABLE();
1457-
}
1458-
result = istrue ? Py_True : Py_False;
1459-
Py_INCREF(result);
1460-
return result;
1445+
Py_RETURN_RICHCOMPARE(diff, 0, op);
14611446
}
14621447

14631448
/* Raises a "can't compare" TypeError and returns NULL. */

‎Modules/_tkinter.c‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -864,13 +864,10 @@ PyTclObject_repr(PyTclObject *self)
864864
return repr;
865865
}
866866

867-
#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
868-
869867
static PyObject *
870868
PyTclObject_richcompare(PyObject *self, PyObject *other, int op)
871869
{
872870
int result;
873-
PyObject *v;
874871

875872
/* neither argument should be NULL, unless something's gone wrong */
876873
if (self == NULL || other == NULL) {
@@ -880,8 +877,7 @@ PyTclObject_richcompare(PyObject *self, PyObject *other, int op)
880877

881878
/* both arguments should be instances of PyTclObject */
882879
if (!PyTclObject_Check(self) || !PyTclObject_Check(other)) {
883-
v = Py_NotImplemented;
884-
goto finished;
880+
Py_RETURN_NOTIMPLEMENTED;
885881
}
886882

887883
if (self == other)
@@ -890,33 +886,7 @@ PyTclObject_richcompare(PyObject *self, PyObject *other, int op)
890886
else
891887
result = strcmp(Tcl_GetString(((PyTclObject *)self)->value),
892888
Tcl_GetString(((PyTclObject *)other)->value));
893-
/* Convert return value to a Boolean */
894-
switch (op) {
895-
case Py_EQ:
896-
v = TEST_COND(result == 0);
897-
break;
898-
case Py_NE:
899-
v = TEST_COND(result != 0);
900-
break;
901-
case Py_LE:
902-
v = TEST_COND(result <= 0);
903-
break;
904-
case Py_GE:
905-
v = TEST_COND(result >= 0);
906-
break;
907-
case Py_LT:
908-
v = TEST_COND(result < 0);
909-
break;
910-
case Py_GT:
911-
v = TEST_COND(result > 0);
912-
break;
913-
default:
914-
PyErr_BadArgument();
915-
return NULL;
916-
}
917-
finished:
918-
Py_INCREF(v);
919-
return v;
889+
Py_RETURN_RICHCOMPARE(result, 0, op);
920890
}
921891

922892
PyDoc_STRVAR(get_typename__doc__, "name of the Tcl type");

‎Modules/parsermodule.c‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,10 @@ parser_compare_nodes(node *left, node *right)
299299
*
300300
*/
301301

302-
#define TEST_COND(cond) ((cond) ? Py_True : Py_False)
303-
304302
static PyObject *
305303
parser_richcompare(PyObject *left, PyObject *right, int op)
306304
{
307305
int result;
308-
PyObject *v;
309306

310307
/* neither argument should be NULL, unless something's gone wrong */
311308
if (left == NULL || right == NULL) {
@@ -315,8 +312,7 @@ parser_richcompare(PyObject *left, PyObject *right, int op)
315312

316313
/* both arguments should be instances of PyST_Object */
317314
if (!PyST_Object_Check(left) || !PyST_Object_Check(right)) {
318-
v = Py_NotImplemented;
319-
goto finished;
315+
Py_RETURN_NOTIMPLEMENTED;
320316
}
321317

322318
if (left == right)
@@ -326,33 +322,7 @@ parser_richcompare(PyObject *left, PyObject *right, int op)
326322
result = parser_compare_nodes(((PyST_Object *)left)->st_node,
327323
((PyST_Object *)right)->st_node);
328324

329-
/* Convert return value to a Boolean */
330-
switch (op) {
331-
case Py_EQ:
332-
v = TEST_COND(result == 0);
333-
break;
334-
case Py_NE:
335-
v = TEST_COND(result != 0);
336-
break;
337-
case Py_LE:
338-
v = TEST_COND(result <= 0);
339-
break;
340-
case Py_GE:
341-
v = TEST_COND(result >= 0);
342-
break;
343-
case Py_LT:
344-
v = TEST_COND(result < 0);
345-
break;
346-
case Py_GT:
347-
v = TEST_COND(result > 0);
348-
break;
349-
default:
350-
PyErr_BadArgument();
351-
return NULL;
352-
}
353-
finished:
354-
Py_INCREF(v);
355-
return v;
325+
Py_RETURN_RICHCOMPARE(result, 0, op);
356326
}
357327

358328
/* parser_newstobject(node* st)

‎Modules/selectmodule.c‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,27 +1929,7 @@ kqueue_event_richcompare(kqueue_event_Object *s, kqueue_event_Object *o,
19291929
: 0;
19301930
#undef CMP
19311931

1932-
switch (op) {
1933-
case Py_EQ:
1934-
result = (result == 0);
1935-
break;
1936-
case Py_NE:
1937-
result = (result != 0);
1938-
break;
1939-
case Py_LE:
1940-
result = (result <= 0);
1941-
break;
1942-
case Py_GE:
1943-
result = (result >= 0);
1944-
break;
1945-
case Py_LT:
1946-
result = (result < 0);
1947-
break;
1948-
case Py_GT:
1949-
result = (result > 0);
1950-
break;
1951-
}
1952-
return PyBool_FromLong((long)result);
1932+
Py_RETURN_RICHCOMPARE(result, 0, op);
19531933
}
19541934

19551935
static PyTypeObject kqueue_event_Type = {

‎Objects/bytearrayobject.c‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,8 +1012,6 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op)
10121012
{
10131013
Py_ssize_t self_size, other_size;
10141014
Py_buffer self_bytes, other_bytes;
1015-
PyObject *res;
1016-
Py_ssize_t minsize;
10171015
int cmp, rc;
10181016

10191017
/* Bytes can be compared to anything that supports the (binary)
@@ -1049,38 +1047,25 @@ bytearray_richcompare(PyObject *self, PyObject *other, int op)
10491047

10501048
if (self_size != other_size && (op == Py_EQ || op == Py_NE)) {
10511049
/* Shortcut: if the lengths differ, the objects differ */
1052-
cmp = (op == Py_NE);
1050+
PyBuffer_Release(&self_bytes);
1051+
PyBuffer_Release(&other_bytes);
1052+
return PyBool_FromLong((op == Py_NE));
10531053
}
10541054
else {
1055-
minsize = self_size;
1056-
if (other_size < minsize)
1057-
minsize = other_size;
1058-
1059-
cmp = memcmp(self_bytes.buf, other_bytes.buf, minsize);
1055+
cmp = memcmp(self_bytes.buf, other_bytes.buf,
1056+
Py_MIN(self_size, other_size));
10601057
/* In ISO C, memcmp() guarantees to use unsigned bytes! */
10611058

1062-
if (cmp == 0) {
1063-
if (self_size < other_size)
1064-
cmp = -1;
1065-
else if (self_size > other_size)
1066-
cmp = 1;
1067-
}
1059+
PyBuffer_Release(&self_bytes);
1060+
PyBuffer_Release(&other_bytes);
10681061

1069-
switch (op) {
1070-
case Py_LT: cmp = cmp < 0; break;
1071-
case Py_LE: cmp = cmp <= 0; break;
1072-
case Py_EQ: cmp = cmp == 0; break;
1073-
case Py_NE: cmp = cmp != 0; break;
1074-
case Py_GT: cmp = cmp > 0; break;
1075-
case Py_GE: cmp = cmp >= 0; break;
1062+
if (cmp != 0) {
1063+
Py_RETURN_RICHCOMPARE(cmp, 0, op);
10761064
}
1065+
1066+
Py_RETURN_RICHCOMPARE(self_size, other_size, op);
10771067
}
10781068

1079-
res = cmp ? Py_True : Py_False;
1080-
PyBuffer_Release(&self_bytes);
1081-
PyBuffer_Release(&other_bytes);
1082-
Py_INCREF(res);
1083-
return res;
10841069
}
10851070

10861071
static void

‎Objects/bytesobject.c‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,7 +1566,6 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
15661566
int c;
15671567
Py_ssize_t len_a, len_b;
15681568
Py_ssize_t min_len;
1569-
PyObject *result;
15701569
int rc;
15711570

15721571
/* Make sure both arguments are strings. */
@@ -1599,20 +1598,20 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
15991598
}
16001599
}
16011600
}
1602-
result = Py_NotImplemented;
1601+
Py_RETURN_NOTIMPLEMENTED;
16031602
}
16041603
else if (a == b) {
16051604
switch (op) {
16061605
case Py_EQ:
16071606
case Py_LE:
16081607
case Py_GE:
16091608
/* a string is equal to itself */
1610-
result = Py_True;
1609+
Py_RETURN_TRUE;
16111610
break;
16121611
case Py_NE:
16131612
case Py_LT:
16141613
case Py_GT:
1615-
result = Py_False;
1614+
Py_RETURN_FALSE;
16161615
break;
16171616
default:
16181617
PyErr_BadArgument();
@@ -1622,7 +1621,7 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
16221621
else if (op == Py_EQ || op == Py_NE) {
16231622
int eq = bytes_compare_eq(a, b);
16241623
eq ^= (op == Py_NE);
1625-
result = eq ? Py_True : Py_False;
1624+
return PyBool_FromLong(eq);
16261625
}
16271626
else {
16281627
len_a = Py_SIZE(a);
@@ -1635,22 +1634,10 @@ bytes_richcompare(PyBytesObject *a, PyBytesObject *b, int op)
16351634
}
16361635
else
16371636
c = 0;
1638-
if (c == 0)
1639-
c = (len_a < len_b) ? -1 : (len_a > len_b) ? 1 : 0;
1640-
switch (op) {
1641-
case Py_LT: c = c < 0; break;
1642-
case Py_LE: c = c <= 0; break;
1643-
case Py_GT: c = c > 0; break;
1644-
case Py_GE: c = c >= 0; break;
1645-
default:
1646-
PyErr_BadArgument();
1647-
return NULL;
1648-
}
1649-
result = c ? Py_True : Py_False;
1637+
if (c != 0)
1638+
Py_RETURN_RICHCOMPARE(c, 0, op);
1639+
Py_RETURN_RICHCOMPARE(len_a, len_b, op);
16501640
}
1651-
1652-
Py_INCREF(result);
1653-
return result;
16541641
}
16551642

16561643
static Py_hash_t

0 commit comments

Comments
 (0)