From 93c1c274f73107be4239c4783372e6f70282e7f3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 1 Aug 2019 15:58:29 -0700 Subject: [PATCH 1/3] bpo-37812: Expand confusing CHECK_SMALL_INT so return is explicit. It's not obvious when looking at a call site of this macro that it affects the control flow. Move the `return` into the callers so it's explicit there, and factor out the fiddly bit as IS_SMALL_INT. As a bonus, a couple of other spots can also use IS_SMALL_INT with a benefit to clarity. --- Objects/longobject.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 3978f5c4a16730..3c41734bb98c83 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -41,6 +41,9 @@ PyObject *_PyLong_One = NULL; -NSMALLNEGINTS (inclusive) to NSMALLPOSINTS (not inclusive). */ static PyLongObject small_ints[NSMALLNEGINTS + NSMALLPOSINTS]; + +#define IS_SMALL_INT(ival) (-NSMALLNEGINTS <= (ival) && (ival) < NSMALLPOSINTS) + #ifdef COUNT_ALLOCS Py_ssize_t _Py_quick_int_allocs, _Py_quick_neg_int_allocs; #endif @@ -49,7 +52,7 @@ static PyObject * get_small_int(sdigit ival) { PyObject *v; - assert(-NSMALLNEGINTS <= ival && ival < NSMALLPOSINTS); + assert(IS_SMALL_INT(ival)); v = (PyObject *)&small_ints[ival + NSMALLNEGINTS]; Py_INCREF(v); #ifdef COUNT_ALLOCS @@ -60,17 +63,13 @@ get_small_int(sdigit ival) #endif return v; } -#define CHECK_SMALL_INT(ival) \ - do if (-NSMALLNEGINTS <= ival && ival < NSMALLPOSINTS) { \ - return get_small_int((sdigit)ival); \ - } while(0) static PyLongObject * maybe_small_long(PyLongObject *v) { if (v && Py_ABS(Py_SIZE(v)) <= 1) { sdigit ival = MEDIUM_VALUE(v); - if (-NSMALLNEGINTS <= ival && ival < NSMALLPOSINTS) { + if (IS_SMALL_INT(ival)) { Py_DECREF(v); return (PyLongObject *)get_small_int(ival); } @@ -78,7 +77,8 @@ maybe_small_long(PyLongObject *v) return v; } #else -#define CHECK_SMALL_INT(ival) +#define IS_SMALL_INT(ival) 0 +#define get_small_int(ival) (assert(0), NULL) #define maybe_small_long(val) (val) #endif @@ -293,7 +293,9 @@ _PyLong_Copy(PyLongObject *src) i = -(i); if (i < 2) { sdigit ival = MEDIUM_VALUE(src); - CHECK_SMALL_INT(ival); + if (IS_SMALL_INT(ival)) { + return get_small_int(ival); + } } result = _PyLong_New(i); if (result != NULL) { @@ -315,7 +317,9 @@ PyLong_FromLong(long ival) int ndigits = 0; int sign; - CHECK_SMALL_INT(ival); + if (IS_SMALL_INT(ival)) { + return get_small_int((sdigit)ival); + } if (ival < 0) { /* negate: can't write this as abs_ival = -ival since that @@ -1146,7 +1150,10 @@ PyLong_FromLongLong(long long ival) int ndigits = 0; int negative = 0; - CHECK_SMALL_INT(ival); + if (IS_SMALL_INT(ival)) { + return get_small_int((sdigit)ival); + } + if (ival < 0) { /* avoid signed overflow on negation; see comments in PyLong_FromLong above. */ @@ -1218,7 +1225,10 @@ PyLong_FromSsize_t(Py_ssize_t ival) int ndigits = 0; int negative = 0; - CHECK_SMALL_INT(ival); + if (IS_SMALL_INT(ival)) { + return get_small_int((sdigit)ival); + } + if (ival < 0) { /* avoid signed overflow when ival = SIZE_T_MIN */ abs_ival = (size_t)(-1-ival)+1; From 27f755548150d92e90c7cd9b1b5828abd6cddd7a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 4 Aug 2019 21:54:14 -0700 Subject: [PATCH 2/3] macro IS_SMALL_INT -> static-inline is_small_int --- Objects/longobject.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 3c41734bb98c83..74037be7a1ea3f 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -42,7 +42,11 @@ PyObject *_PyLong_One = NULL; */ static PyLongObject small_ints[NSMALLNEGINTS + NSMALLPOSINTS]; -#define IS_SMALL_INT(ival) (-NSMALLNEGINTS <= (ival) && (ival) < NSMALLPOSINTS) +static inline int +is_small_int(long long ival) +{ + return -NSMALLNEGINTS <= ival && ival < NSMALLPOSINTS; +} #ifdef COUNT_ALLOCS Py_ssize_t _Py_quick_int_allocs, _Py_quick_neg_int_allocs; @@ -52,7 +56,7 @@ static PyObject * get_small_int(sdigit ival) { PyObject *v; - assert(IS_SMALL_INT(ival)); + assert(is_small_int(ival)); v = (PyObject *)&small_ints[ival + NSMALLNEGINTS]; Py_INCREF(v); #ifdef COUNT_ALLOCS @@ -69,7 +73,7 @@ maybe_small_long(PyLongObject *v) { if (v && Py_ABS(Py_SIZE(v)) <= 1) { sdigit ival = MEDIUM_VALUE(v); - if (IS_SMALL_INT(ival)) { + if (is_small_int(ival)) { Py_DECREF(v); return (PyLongObject *)get_small_int(ival); } @@ -77,7 +81,7 @@ maybe_small_long(PyLongObject *v) return v; } #else -#define IS_SMALL_INT(ival) 0 +#define is_small_int(ival) 0 #define get_small_int(ival) (assert(0), NULL) #define maybe_small_long(val) (val) #endif @@ -293,7 +297,7 @@ _PyLong_Copy(PyLongObject *src) i = -(i); if (i < 2) { sdigit ival = MEDIUM_VALUE(src); - if (IS_SMALL_INT(ival)) { + if (is_small_int(ival)) { return get_small_int(ival); } } @@ -317,7 +321,7 @@ PyLong_FromLong(long ival) int ndigits = 0; int sign; - if (IS_SMALL_INT(ival)) { + if (is_small_int(ival)) { return get_small_int((sdigit)ival); } @@ -1150,7 +1154,7 @@ PyLong_FromLongLong(long long ival) int ndigits = 0; int negative = 0; - if (IS_SMALL_INT(ival)) { + if (is_small_int(ival)) { return get_small_int((sdigit)ival); } @@ -1225,7 +1229,7 @@ PyLong_FromSsize_t(Py_ssize_t ival) int ndigits = 0; int negative = 0; - if (IS_SMALL_INT(ival)) { + if (is_small_int(ival)) { return get_small_int((sdigit)ival); } From 9b210ed65bb9c127cd42233332083955cf5797f8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 23 Aug 2019 22:46:44 -0700 Subject: [PATCH 3/3] Add NEWS entry. --- .../Core and Builtins/2019-08-23-22-46-25.bpo-37812.vsWZwS.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-08-23-22-46-25.bpo-37812.vsWZwS.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-08-23-22-46-25.bpo-37812.vsWZwS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-08-23-22-46-25.bpo-37812.vsWZwS.rst new file mode 100644 index 00000000000000..90c3ff3f7ef9ce --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-08-23-22-46-25.bpo-37812.vsWZwS.rst @@ -0,0 +1,3 @@ +The ``CHECK_SMALL_INT`` macro used inside :file:`Object/longobject.c` has +been replaced with an explicit ``return`` at each call site, conditioned on +a ``static inline`` function ``is_small_int``.