Skip to content

Commit e2c3c7e

Browse files
authored
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.
1 parent 2423814 commit e2c3c7e

2 files changed

Lines changed: 66 additions & 8 deletions

File tree

‎Include/pymacro.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,20 @@
118118
/* Minimum value between x and y */
119119
# define Py_MIN(x, y) \
120120
__extension__ \
121-
({ _Py_TYPEOF (x) _x = (x); \
122-
_Py_TYPEOF (y) _y = (y); \
123-
_x < _y ? _x : _y; })
121+
({ _Py_TYPEOF (x) _PyMIN_x = (x); \
122+
_Py_TYPEOF (y) _PyMIN_y = (y); \
123+
_PyMIN_x < _PyMIN_y ? _PyMIN_x : _PyMIN_y; })
124124
/* Maximum value between x and y */
125125
# define Py_MAX(x, y) \
126126
__extension__ \
127-
({ _Py_TYPEOF (x) _x = (x); \
128-
_Py_TYPEOF (y) _y = (y); \
129-
_x > _y ? _x : _y; })
127+
({ _Py_TYPEOF (x) _PyMAX_x = (x); \
128+
_Py_TYPEOF (y) _PyMAX_y = (y); \
129+
_PyMAX_x > _PyMAX_y ? _PyMAX_x : _PyMAX_y; })
130130
/* Absolute value of the number x */
131131
# define Py_ABS(x) \
132132
__extension__ \
133-
({ _Py_TYPEOF (x) _x = (x); \
134-
_x < 0 ? -_x : _x; })
133+
({ _Py_TYPEOF (x) _PyABS_x = (x); \
134+
_PyABS_x < 0 ? -_PyABS_x : _PyABS_x; })
135135
#else
136136
/* Minimum value between x and y */
137137
# define Py_MIN(x, y) (((x) > (y)) ? (y) : (x))

‎Modules/_testlimitedcapi/object.c‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,63 @@ test_refcount_macros(PyObject *self, PyObject *Py_UNUSED(ignored))
241241
TEST_REFCOUNT();
242242
}
243243

244+
245+
// Test Py_MIN(), Py_MAX() and Py_ABS() macros.
246+
// On GCC/clang, they are implemented as a statement expression to only
247+
// evaluate each argument only once.
248+
static PyObject*
249+
test_min_max_abs_macros(PyObject *self, PyObject *Py_UNUSED(ignored))
250+
{
251+
int x = 5, y = 7, z = -11;
252+
253+
// Simple usage
254+
assert(Py_MIN(x, y) == 5);
255+
assert(Py_MAX(x, y) == 7);
256+
assert(Py_ABS(z) == 11);
257+
258+
// Combined macros
259+
assert(Py_MIN(x, Py_MIN(y, z)) == -11);
260+
assert(Py_MAX(x, Py_MAX(y, z)) == 7);
261+
assert(Py_MIN(x, Py_MAX(y, z)) == 5);
262+
assert(Py_MAX(x, Py_MIN(y, z)) == 5);
263+
assert(Py_ABS(Py_ABS(z)) == 11);
264+
265+
// A few more tests
266+
assert(Py_MIN(2, Py_MIN(6, 12)) == 2);
267+
assert(Py_MAX(2, Py_MAX(6, 12)) == 12);
268+
assert(Py_MIN(2, Py_MAX(6, 12)) == 2);
269+
assert(Py_MAX(2, Py_MIN(6, 12)) == 6);
270+
assert(Py_MIN(Py_MIN(2, 6), 12) == 2);
271+
assert(Py_MAX(Py_MAX(2, 6), 12) == 12);
272+
assert(Py_MIN(Py_MAX(2, 6), 12) == 6);
273+
assert(Py_MAX(Py_MIN(2, 6), 12) == 12);
274+
assert(Py_ABS(Py_ABS(12)) == 12);
275+
assert(Py_ABS(Py_ABS(12)) == 12);
276+
277+
// Integer limits
278+
assert(Py_MIN(123, INT_MIN) == INT_MIN);
279+
assert(Py_MAX(123, INT_MAX) == INT_MAX);
280+
assert(Py_ABS(INT_MAX) == INT_MAX);
281+
// Do not tests Py_ABS(INT_MIN), since the behavior is documented.
282+
// This limitation is documented in Py_ABS() documentation.
283+
284+
#if ((defined(__GNUC__) || defined(__clang__)) \
285+
&& defined(_Py_TYPEOF) && !defined(__cplusplus))
286+
// Check that arguments are only evaluated once
287+
int a = 5, b = 7, c = -11;
288+
assert(Py_MIN(++a, ++b) == 6);
289+
assert(a == 6);
290+
assert(b == 8);
291+
assert(Py_MAX(++a, ++b) == 9);
292+
assert(a == 7);
293+
assert(b == 9);
294+
assert(Py_ABS(--c) == 12);
295+
assert(c == -12);
296+
#endif
297+
298+
Py_RETURN_NONE;
299+
}
300+
244301
#undef Py_NewRef
245302
#undef Py_XNewRef
246303

@@ -315,6 +372,7 @@ static PyMethodDef test_methods[] = {
315372
{"test_py_setref", test_py_setref, METH_NOARGS},
316373
{"test_refcount_macros", test_refcount_macros, METH_NOARGS},
317374
{"test_refcount_funcs", test_refcount_funcs, METH_NOARGS},
375+
{"test_min_max_abs_macros", test_min_max_abs_macros, METH_NOARGS},
318376
{"test_py_is_macros", test_py_is_macros, METH_NOARGS},
319377
{"test_py_is_funcs", test_py_is_funcs, METH_NOARGS},
320378
{NULL},

0 commit comments

Comments
 (0)