Issue #16451: Refactor to remove duplication between range and slice in slice index computations.

3 files changed, 87 insertions, 269 deletions

diff --git a/Include/sliceobject.h b/Include/sliceobject.h
index 8bec179..f7ee90c 100644
--- a/Include/sliceobject.h
+++ b/Include/sliceobject.h
@@ -34,6 +34,9 @@ PyAPI_FUNC(PyObject *) PySlice_New(PyObject* start, PyObject* stop,
                                   PyObject* step);
 #ifndef Py_LIMITED_API
 PyAPI_FUNC(PyObject *) _PySlice_FromIndices(Py_ssize_t start, Py_ssize_t stop);
+PyAPI_FUNC(int) _PySlice_GetLongIndices(PySliceObject *self, PyObject *length,
+                                 PyObject **start_ptr, PyObject **stop_ptr,
+                                 PyObject **step_ptr);
 #endif
 PyAPI_FUNC(int) PySlice_GetIndices(PyObject *r, Py_ssize_t length,
                                   Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t *step);
diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c
index 214b455..ba51fec 100644
--- a/Objects/rangeobject.c
+++ b/Objects/rangeobject.c
@@ -318,195 +318,6 @@ range_item(rangeobject *r, Py_ssize_t i)
     return res;
 }
 
-/* Additional helpers, since the standard slice helpers
- * all clip to PY_SSIZE_T_MAX
- */
-
-/* Replace _PyEval_SliceIndex */
-static PyObject *
-compute_slice_element(PyObject *obj)
-{
-    PyObject *result = NULL;
-    if (obj != NULL) {
-        if (PyIndex_Check(obj)) {
-            result = PyNumber_Index(obj);
-        }
-        else {
-            PyErr_SetString(PyExc_TypeError,
-                            "slice indices must be integers or "
-                            "None or have an __index__ method");
-        }
-    }
-    return result;
-}
-
-/* Replace PySlice_GetIndicesEx
- *   Result indicates whether or not the slice is empty
- *    (-1 = error, 0 = empty slice, 1 = slice contains elements)
- */
-static int
-compute_slice_indices(rangeobject *r, PySliceObject *slice,
-                      PyObject **start, PyObject **stop, PyObject **step)
-{
-    int cmp_result, has_elements;
-    Py_ssize_t clamped_step = 0;
-    PyObject *zero = NULL, *one = NULL, *neg_one = NULL, *candidate = NULL;
-    PyObject *tmp_start = NULL, *tmp_stop = NULL, *tmp_step = NULL;
-    zero = PyLong_FromLong(0);
-    if (zero == NULL) goto Fail;
-    one = PyLong_FromLong(1);
-    if (one == NULL) goto Fail;
-    neg_one = PyLong_FromLong(-1);
-    if (neg_one == NULL) goto Fail;
-
-    /* Calculate step value */
-    if (slice->step == Py_None) {
-        clamped_step = 1;
-        tmp_step = one;
-        Py_INCREF(tmp_step);
-    } else {
-        if (!_PyEval_SliceIndex(slice->step, &clamped_step)) goto Fail;
-        if (clamped_step == 0) {
-            PyErr_SetString(PyExc_ValueError,
-                            "slice step cannot be zero");
-            goto Fail;
-        }
-        tmp_step = compute_slice_element(slice->step);
-        if (tmp_step == NULL) goto Fail;
-    }
-
-    /* Calculate start value */
-    if (slice->start == Py_None) {
-        if (clamped_step < 0) {
-            tmp_start = PyNumber_Subtract(r->length, one);
-            if (tmp_start == NULL) goto Fail;
-        } else {
-            tmp_start = zero;
-            Py_INCREF(tmp_start);
-        }
-    } else {
-        candidate = compute_slice_element(slice->start);
-        if (candidate == NULL) goto Fail;
-        cmp_result = PyObject_RichCompareBool(candidate, zero, Py_LT);
-        if (cmp_result == -1) goto Fail;
-        if (cmp_result) {
-            /* candidate < 0 */
-            tmp_start = PyNumber_Add(r->length, candidate);
-            if (tmp_start == NULL) goto Fail;
-            Py_CLEAR(candidate);
-        } else {
-            /* candidate >= 0 */
-            tmp_start = candidate;
-            candidate = NULL;
-        }
-        cmp_result = PyObject_RichCompareBool(tmp_start, zero, Py_LT);
-        if (cmp_result == -1) goto Fail;
-        if (cmp_result) {
-            /* tmp_start < 0 */
-            Py_CLEAR(tmp_start);
-            if (clamped_step < 0) {
-                tmp_start = neg_one;
-            } else {
-                tmp_start = zero;
-            }
-            Py_INCREF(tmp_start);
-        } else {
-            /* tmp_start >= 0 */
-            cmp_result = PyObject_RichCompareBool(tmp_start, r->length, Py_GE);
-            if (cmp_result == -1) goto Fail;
-            if (cmp_result) {
-                /* tmp_start >= r->length */
-                Py_CLEAR(tmp_start);
-                if (clamped_step < 0) {
-                    tmp_start = PyNumber_Subtract(r->length, one);
-                    if (tmp_start == NULL) goto Fail;
-                } else {
-                    tmp_start = r->length;
-                    Py_INCREF(tmp_start);
-                }
-            }
-        }
-    }
-
-    /* Calculate stop value */
-    if (slice->stop == Py_None) {
-        if (clamped_step < 0) {
-            tmp_stop = neg_one;
-        } else {
-            tmp_stop = r->length;
-        }
-        Py_INCREF(tmp_stop);
-    } else {
-        candidate = compute_slice_element(slice->stop);
-        if (candidate == NULL) goto Fail;
-        cmp_result = PyObject_RichCompareBool(candidate, zero, Py_LT);
-        if (cmp_result == -1) goto Fail;
-        if (cmp_result) {
-            /* candidate < 0 */
-            tmp_stop = PyNumber_Add(r->length, candidate);
-            if (tmp_stop == NULL) goto Fail;
-            Py_CLEAR(candidate);
-        } else {
-            /* candidate >= 0 */
-            tmp_stop = candidate;
-            candidate = NULL;
-        }
-        cmp_result = PyObject_RichCompareBool(tmp_stop, zero, Py_LT);
-        if (cmp_result == -1) goto Fail;
-        if (cmp_result) {
-            /* tmp_stop < 0 */
-            Py_CLEAR(tmp_stop);
-            if (clamped_step < 0) {
-                tmp_stop = neg_one;
-            } else {
-                tmp_stop = zero;
-            }
-            Py_INCREF(tmp_stop);
-        } else {
-            /* tmp_stop >= 0 */
-            cmp_result = PyObject_RichCompareBool(tmp_stop, r->length, Py_GE);
-            if (cmp_result == -1) goto Fail;
-            if (cmp_result) {
-                /* tmp_stop >= r->length */
-                Py_CLEAR(tmp_stop);
-                if (clamped_step < 0) {
-                    tmp_stop = PyNumber_Subtract(r->length, one);
-                    if (tmp_stop == NULL) goto Fail;
-                } else {
-                    tmp_stop = r->length;
-                    Py_INCREF(tmp_stop);
-                }
-            }
-        }
-    }
-
-    /* Check if the slice is empty or not */
-    if (clamped_step < 0) {
-        has_elements = PyObject_RichCompareBool(tmp_start, tmp_stop, Py_GT);
-    } else {
-        has_elements = PyObject_RichCompareBool(tmp_start, tmp_stop, Py_LT);
-    }
-    if (has_elements == -1) goto Fail;
-
-    *start = tmp_start;
-    *stop = tmp_stop;
-    *step = tmp_step;
-    Py_DECREF(neg_one);
-    Py_DECREF(one);
-    Py_DECREF(zero);
-    return has_elements;
-
-  Fail:
-    Py_XDECREF(tmp_start);
-    Py_XDECREF(tmp_stop);
-    Py_XDECREF(tmp_step);
-    Py_XDECREF(candidate);
-    Py_XDECREF(neg_one);
-    Py_XDECREF(one);
-    Py_XDECREF(zero);
-    return -1;
-}
-
 static PyObject *
 compute_slice(rangeobject *r, PyObject *_slice)
 {
@@ -514,10 +325,11 @@ compute_slice(rangeobject *r, PyObject *_slice)
     rangeobject *result;
     PyObject *start = NULL, *stop = NULL, *step = NULL;
     PyObject *substart = NULL, *substop = NULL, *substep = NULL;
-    int has_elements;
+    int error;
 
-    has_elements = compute_slice_indices(r, slice, &start, &stop, &step);
-    if (has_elements == -1) return NULL;
+    error = _PySlice_GetLongIndices(slice, r->length, &start, &stop, &step);
+    if (error == -1)
+        return NULL;
 
     substep = PyNumber_Multiply(r->step, step);
     if (substep == NULL) goto fail;
@@ -527,13 +339,8 @@ compute_slice(rangeobject *r, PyObject *_slice)
     if (substart == NULL) goto fail;
     Py_CLEAR(start);
 
-    if (has_elements) {
-        substop = compute_item(r, stop);
-        if (substop == NULL) goto fail;
-    } else {
-        substop = substart;
-        Py_INCREF(substop);
-    }
+    substop = compute_item(r, stop);
+    if (substop == NULL) goto fail;
     Py_CLEAR(stop);
 
     result = make_range_object(Py_TYPE(r), substart, substop, substep);
diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c
index 4b31f23..52f1c89 100644
--- a/Objects/sliceobject.c
+++ b/Objects/sliceobject.c
@@ -316,57 +316,41 @@ evaluate_slice_index(PyObject *v)
     }
 }
 
-/* Implementation of slice.indices. */
+/* Compute slice indices given a slice and length.  Return -1 on failure.  Used
+   by slice.indices and rangeobject slicing.  Assumes that `len` is a
+   nonnegative instance of PyLong. */
 
-static PyObject*
-slice_indices(PySliceObject* self, PyObject* len)
+int
+_PySlice_GetLongIndices(PySliceObject *self, PyObject *length,
+                        PyObject **start_ptr, PyObject **stop_ptr,
+                        PyObject **step_ptr)
 {
     PyObject *start=NULL, *stop=NULL, *step=NULL;
-    PyObject *length=NULL, *upper=NULL, *lower=NULL, *zero=NULL;
-    int step_is_negative, cmp;
-
-    zero = PyLong_FromLong(0L);
-    if (zero == NULL)
-        return NULL;
-
-    /* Compute step and length as integers. */
-    length = PyNumber_Index(len);
-    if (length == NULL)
-        goto error;
+    PyObject *upper=NULL, *lower=NULL;
+    int step_is_negative, cmp_result;
 
-    if (self->step == Py_None)
+    /* Convert step to an integer; raise for zero step. */
+    if (self->step == Py_None) {
         step = PyLong_FromLong(1L);
-    else
-        step = evaluate_slice_index(self->step);
-    if (step == NULL)
-        goto error;
-
-    /* Raise ValueError for negative length or zero step. */
-    cmp = PyObject_RichCompareBool(length, zero, Py_LT);
-    if (cmp < 0) {
-        goto error;
-    }
-    if (cmp) {
-        PyErr_SetString(PyExc_ValueError,
-                        "length should not be negative");
-        goto error;
-    }
-
-    cmp = PyObject_RichCompareBool(step, zero, Py_EQ);
-    if (cmp < 0) {
-        goto error;
+        if (step == NULL)
+            goto error;
+        step_is_negative = 0;
     }
-    if (cmp) {
-        PyErr_SetString(PyExc_ValueError,
-                        "slice step cannot be zero");
-        goto error;
+    else {
+        int step_sign;
+        step = evaluate_slice_index(self->step);
+        if (step == NULL)
+            goto error;
+        step_sign = _PyLong_Sign(step);
+        if (step_sign == 0) {
+            PyErr_SetString(PyExc_ValueError,
+                            "slice step cannot be zero");
+            goto error;
+        }
+        step_is_negative = step_sign < 0;
     }
 
     /* Find lower and upper bounds for start and stop. */
-    step_is_negative = PyObject_RichCompareBool(step, zero, Py_LT);
-    if (step_is_negative < 0) {
-        goto error;
-    }
     if (step_is_negative) {
         lower = PyLong_FromLong(-1L);
         if (lower == NULL)
@@ -377,8 +361,10 @@ slice_indices(PySliceObject* self, PyObject* len)
             goto error;
     }
     else {
-        lower = zero;
-        Py_INCREF(lower);
+        lower = PyLong_FromLong(0L);
+        if (lower == NULL)
+            goto error;
+
         upper = length;
         Py_INCREF(upper);
     }
@@ -393,10 +379,7 @@ slice_indices(PySliceObject* self, PyObject* len)
         if (start == NULL)
             goto error;
 
-        cmp = PyObject_RichCompareBool(start, zero, Py_LT);
-        if (cmp < 0)
-            goto error;
-        if (cmp) {
+        if (_PyLong_Sign(start) < 0) {
             /* start += length */
             PyObject *tmp = PyNumber_Add(start, length);
             Py_DECREF(start);
@@ -404,20 +387,20 @@ slice_indices(PySliceObject* self, PyObject* len)
             if (start == NULL)
                 goto error;
 
-            cmp = PyObject_RichCompareBool(start, lower, Py_LT);
-            if (cmp < 0)
+            cmp_result = PyObject_RichCompareBool(start, lower, Py_LT);
+            if (cmp_result < 0)
                 goto error;
-            if (cmp) {
+            if (cmp_result) {
                 Py_INCREF(lower);
                 Py_DECREF(start);
                 start = lower;
             }
         }
         else {
-            cmp = PyObject_RichCompareBool(start, upper, Py_GT);
-            if (cmp < 0)
+            cmp_result = PyObject_RichCompareBool(start, upper, Py_GT);
+            if (cmp_result < 0)
                 goto error;
-            if (cmp) {
+            if (cmp_result) {
                 Py_INCREF(upper);
                 Py_DECREF(start);
                 start = upper;
@@ -435,10 +418,7 @@ slice_indices(PySliceObject* self, PyObject* len)
         if (stop == NULL)
             goto error;
 
-        cmp = PyObject_RichCompareBool(stop, zero, Py_LT);
-        if (cmp < 0)
-            goto error;
-        if (cmp) {
+        if (_PyLong_Sign(stop) < 0) {
             /* stop += length */
             PyObject *tmp = PyNumber_Add(stop, length);
             Py_DECREF(stop);
@@ -446,20 +426,20 @@ slice_indices(PySliceObject* self, PyObject* len)
             if (stop == NULL)
                 goto error;
 
-            cmp = PyObject_RichCompareBool(stop, lower, Py_LT);
-            if (cmp < 0)
+            cmp_result = PyObject_RichCompareBool(stop, lower, Py_LT);
+            if (cmp_result < 0)
                 goto error;
-            if (cmp) {
+            if (cmp_result) {
                 Py_INCREF(lower);
                 Py_DECREF(stop);
                 stop = lower;
             }
         }
         else {
-            cmp = PyObject_RichCompareBool(stop, upper, Py_GT);
-            if (cmp < 0)
+            cmp_result = PyObject_RichCompareBool(stop, upper, Py_GT);
+            if (cmp_result < 0)
                 goto error;
-            if (cmp) {
+            if (cmp_result) {
                 Py_INCREF(upper);
                 Py_DECREF(stop);
                 stop = upper;
@@ -467,23 +447,51 @@ slice_indices(PySliceObject* self, PyObject* len)
         }
     }
 
+    *start_ptr = start;
+    *stop_ptr = stop;
+    *step_ptr = step;
     Py_DECREF(upper);
     Py_DECREF(lower);
-    Py_DECREF(length);
-    Py_DECREF(zero);
-    return Py_BuildValue("(NNN)", start, stop, step);
+    return 0;
 
   error:
+    *start_ptr = *stop_ptr = *step_ptr = NULL;
     Py_XDECREF(start);
     Py_XDECREF(stop);
     Py_XDECREF(step);
     Py_XDECREF(upper);
     Py_XDECREF(lower);
-    Py_XDECREF(length);
-    Py_XDECREF(zero);
-    return NULL;
+    return -1;
 }
 
+/* Implementation of slice.indices. */
+
+static PyObject*
+slice_indices(PySliceObject* self, PyObject* len)
+{
+    PyObject *start, *stop, *step;
+    PyObject *length;
+    int error;
+
+    /* Convert length to an integer if necessary; raise for negative length. */
+    length = PyNumber_Index(len);
+    if (length == NULL)
+        return NULL;
+
+    if (_PyLong_Sign(length) < 0) {
+        PyErr_SetString(PyExc_ValueError,
+                        "length should not be negative");
+        Py_DECREF(length);
+        return NULL;
+    }
+
+    error = _PySlice_GetLongIndices(self, length, &start, &stop, &step);
+    Py_DECREF(length);
+    if (error == -1)
+        return NULL;
+    else
+        return Py_BuildValue("(NNN)", start, stop, step);
+}
 
 PyDoc_STRVAR(slice_indices_doc,
 "S.indices(len) -> (start, stop, stride)\n\