gh-158942: Use more unique variable names in Py_MIN/MAX/ABS() (#158969)
If Py_MIN/MAX/ABS() macros are called on an existing "_x" or "_y"
variable name, the external variable is used instead of macro local
variable. So use more unique names in the macros to avoid
the issue.
Add test_min_max_abs_macros() to _testlimitedcapi.
diff --git a/Include/pymacro.h b/Include/pymacro.h
index 122d263..41dfd91 100644
--- a/Include/pymacro.h
+++ b/Include/pymacro.h
@@ -118,20 +118,20 @@
/* Minimum value between x and y */
# define Py_MIN(x, y) \
__extension__ \
- ({ _Py_TYPEOF (x) _x = (x); \
- _Py_TYPEOF (y) _y = (y); \
- _x < _y ? _x : _y; })
+ ({ _Py_TYPEOF (x) _PyMIN_x = (x); \
+ _Py_TYPEOF (y) _PyMIN_y = (y); \
+ _PyMIN_x < _PyMIN_y ? _PyMIN_x : _PyMIN_y; })
/* Maximum value between x and y */
# define Py_MAX(x, y) \
__extension__ \
- ({ _Py_TYPEOF (x) _x = (x); \
- _Py_TYPEOF (y) _y = (y); \
- _x > _y ? _x : _y; })
+ ({ _Py_TYPEOF (x) _PyMAX_x = (x); \
+ _Py_TYPEOF (y) _PyMAX_y = (y); \
+ _PyMAX_x > _PyMAX_y ? _PyMAX_x : _PyMAX_y; })
/* Absolute value of the number x */
# define Py_ABS(x) \
__extension__ \
- ({ _Py_TYPEOF (x) _x = (x); \
- _x < 0 ? -_x : _x; })
+ ({ _Py_TYPEOF (x) _PyABS_x = (x); \
+ _PyABS_x < 0 ? -_PyABS_x : _PyABS_x; })
#else
/* Minimum value between x and y */
# define Py_MIN(x, y) (((x) > (y)) ? (y) : (x))
diff --git a/Modules/_testlimitedcapi/object.c b/Modules/_testlimitedcapi/object.c
index 03debb5..5eee2a9 100644
--- a/Modules/_testlimitedcapi/object.c
+++ b/Modules/_testlimitedcapi/object.c
@@ -241,6 +241,63 @@ test_refcount_macros(PyObject *self, PyObject *Py_UNUSED(ignored))
TEST_REFCOUNT();
}
+
+// Test Py_MIN(), Py_MAX() and Py_ABS() macros.
+// On GCC/clang, they are implemented as a statement expression to only
+// evaluate each argument only once.
+static PyObject*
+test_min_max_abs_macros(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+ int x = 5, y = 7, z = -11;
+
+ // Simple usage
+ assert(Py_MIN(x, y) == 5);
+ assert(Py_MAX(x, y) == 7);
+ assert(Py_ABS(z) == 11);
+
+ // Combined macros
+ assert(Py_MIN(x, Py_MIN(y, z)) == -11);
+ assert(Py_MAX(x, Py_MAX(y, z)) == 7);
+ assert(Py_MIN(x, Py_MAX(y, z)) == 5);
+ assert(Py_MAX(x, Py_MIN(y, z)) == 5);
+ assert(Py_ABS(Py_ABS(z)) == 11);
+
+ // A few more tests
+ assert(Py_MIN(2, Py_MIN(6, 12)) == 2);
+ assert(Py_MAX(2, Py_MAX(6, 12)) == 12);
+ assert(Py_MIN(2, Py_MAX(6, 12)) == 2);
+ assert(Py_MAX(2, Py_MIN(6, 12)) == 6);
+ assert(Py_MIN(Py_MIN(2, 6), 12) == 2);
+ assert(Py_MAX(Py_MAX(2, 6), 12) == 12);
+ assert(Py_MIN(Py_MAX(2, 6), 12) == 6);
+ assert(Py_MAX(Py_MIN(2, 6), 12) == 12);
+ assert(Py_ABS(Py_ABS(12)) == 12);
+ assert(Py_ABS(Py_ABS(12)) == 12);
+
+ // Integer limits
+ assert(Py_MIN(123, INT_MIN) == INT_MIN);
+ assert(Py_MAX(123, INT_MAX) == INT_MAX);
+ assert(Py_ABS(INT_MAX) == INT_MAX);
+ // Do not tests Py_ABS(INT_MIN), since the behavior is documented.
+ // This limitation is documented in Py_ABS() documentation.
+
+#if ((defined(__GNUC__) || defined(__clang__)) \
+ && defined(_Py_TYPEOF) && !defined(__cplusplus))
+ // Check that arguments are only evaluated once
+ int a = 5, b = 7, c = -11;
+ assert(Py_MIN(++a, ++b) == 6);
+ assert(a == 6);
+ assert(b == 8);
+ assert(Py_MAX(++a, ++b) == 9);
+ assert(a == 7);
+ assert(b == 9);
+ assert(Py_ABS(--c) == 12);
+ assert(c == -12);
+#endif
+
+ Py_RETURN_NONE;
+}
+
#undef Py_NewRef
#undef Py_XNewRef
@@ -315,6 +372,7 @@ static PyMethodDef test_methods[] = {
{"test_py_setref", test_py_setref, METH_NOARGS},
{"test_refcount_macros", test_refcount_macros, METH_NOARGS},
{"test_refcount_funcs", test_refcount_funcs, METH_NOARGS},
+ {"test_min_max_abs_macros", test_min_max_abs_macros, METH_NOARGS},
{"test_py_is_macros", test_py_is_macros, METH_NOARGS},
{"test_py_is_funcs", test_py_is_funcs, METH_NOARGS},
{NULL},
