Apply first round of code revew comments

python · brandtbucher · Jan 11, 2022 · Jan 12, 2022 · Jan 12, 2022 · Jan 12, 2022
commit 6b4cc6dabf5e0350ea62aa874e625add8da2eca3
diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h
@@ -36,10 +36,6 @@ static inline PyObject* _PyLong_GetZero(void)
 static inline PyObject* _PyLong_GetOne(void)
 { return (PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS+1]; }
 
-PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right);
-PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right);
-PyObject *_PyLong_Subtract(PyLongObject *left, PyLongObject *right);
-
 /* Used by Python/mystrtoul.c, _PyBytes_FromHex(),
    _PyBytes_DecodeEscape(), etc. */
 PyAPI_DATA(unsigned char) _PyLong_DigitValue[256];

diff --git a/Objects/longobject.c b/Objects/longobject.c
@@ -3139,9 +3139,10 @@ x_sub(PyLongObject *a, PyLongObject *b)
     return maybe_small_long(long_normalize(z));
 }
 
-PyObject *
-_PyLong_Add(PyLongObject *a, PyLongObject *b)
+static PyObject *
+long_add(PyLongObject *a, PyLongObject *b)
 {
+    CHECK_BINOP(a, b);
     if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) {
         return _PyLong_FromSTwoDigits(medium_value(a) + medium_value(b));
     }
@@ -3172,20 +3173,14 @@ _PyLong_Add(PyLongObject *a, PyLongObject *b)
 }
 
 static PyObject *
-long_add(PyLongObject *a, PyLongObject *b)
+long_sub(PyLongObject *a, PyLongObject *b)
 {
     CHECK_BINOP(a, b);
-    return _PyLong_Add(a, b);
-}
-
-PyObject *
-_PyLong_Subtract(PyLongObject *a, PyLongObject *b)
-{
-    PyLongObject *z;
-
     if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) {
         return _PyLong_FromSTwoDigits(medium_value(a) - medium_value(b));
     }
+
+    PyLongObject *z;
     if (Py_SIZE(a) < 0) {
         if (Py_SIZE(b) < 0) {
             z = x_sub(b, a);
@@ -3207,13 +3202,6 @@ _PyLong_Subtract(PyLongObject *a, PyLongObject *b)
     return (PyObject *)z;
 }
 
-static PyObject *
-long_sub(PyLongObject *a, PyLongObject *b)
-{
-    CHECK_BINOP(a, b);
-    return _PyLong_Subtract(a, b);
-}
-
 /* Grade school multiplication, ignoring the signs.
  * Returns the absolute value of the product, or NULL if error.
  */
@@ -3631,18 +3619,17 @@ k_lopsided_mul(PyLongObject *a, PyLongObject *b)
     return NULL;
 }
 
-PyObject *
-_PyLong_Multiply(PyLongObject *a, PyLongObject *b)
+static PyObject *
+long_mul(PyLongObject *a, PyLongObject *b)
 {
-    PyLongObject *z;
-
+    CHECK_BINOP(a, b);
     /* fast path for single-digit multiplication */
     if (IS_MEDIUM_VALUE(a) && IS_MEDIUM_VALUE(b)) {
         stwodigits v = medium_value(a) * medium_value(b);
         return _PyLong_FromSTwoDigits(v);
     }
 
-    z = k_mul(a, b);
+    PyLongObject *z = k_mul(a, b);
     /* Negate if exactly one of the inputs is negative. */
     if (((Py_SIZE(a) ^ Py_SIZE(b)) < 0) && z) {
         _PyLong_Negate(&z);
@@ -3652,13 +3639,6 @@ _PyLong_Multiply(PyLongObject *a, PyLongObject *b)
     return (PyObject *)z;
 }
 
-static PyObject *
-long_mul(PyLongObject *a, PyLongObject *b)
-{
-    CHECK_BINOP(a, b);
-    return _PyLong_Multiply(a, b);
-}
-
 /* Fast modulo division for single-digit longs. */
 static PyObject *
 fast_mod(PyLongObject *a, PyLongObject *b)

@@ -885,15 +885,18 @@ static const binaryfunc binary_ops[] = {
         }                                                                      \
         bool inplace = NEXT_OP_STORES(lhs);                                    \
         if (Py_ABS(i) < PyLong_BASE && Py_REFCNT(lhs) == inplace + 1) {        \
-            assert(lhs < (PyObject *)&_PyLong_SMALL_INTS[0] ||                 \
-                   lhs >= (PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS +  \
-                                                          _PY_NSMALLPOSINTS]); \
-            ((PyLongObject *)lhs)->ob_digit[0] = Py_ABS(i);                    \
+            /* If this assert fails, it's probably one of two things:       */ \
+            /* - If lhs lives in _PyLong_SMALL_INTS, its refcount is wrong. */ \
+            /* - Otherwise, whatever created lhs should have used           */ \
+            /*   _PyLong_SMALL_INTS, but didn't (see bpo-46361 for an       */ \
+            /*   example of this.)                                          */ \
+            assert(l < -_PY_NSMALLNEGINTS || _PY_NSMALLPOSINTS <= l);          \
+            ((PyLongObject *)lhs)->ob_digit[0] = (digit)Py_ABS(i);             \
             Py_SET_SIZE(lhs, i < 0 ? -1 : 1);                                  \
             DISPATCH();                                                        \
         }                                                                      \
         Py_DECREF(lhs);                                                        \
-        PyObject *res = PyLong_FromLong(i);                                    \
+        PyObject *res = PyLong_FromLongLong(i);                                \
         SET_TOP(res);                                                          \
         if (res == NULL) {                                                     \
             goto error;                                                        \