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

1 files changed, 78 insertions, 70 deletions

diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c
index 4b31f2306e..52f1c89ded 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\