Skip to content

Commit ffdb2c2

Browse files
mdickinsonmdickinson
committed
Issue python#16451: Refactor to remove duplication between range and slice in slice index computations.
1 parent c3afba1 commit ffdb2c2

3 files changed

Lines changed: 87 additions & 269 deletions

File tree

‎Include/sliceobject.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ PyAPI_FUNC(PyObject *) PySlice_New(PyObject* start, PyObject* stop,
3434
PyObject* step);
3535
#ifndef Py_LIMITED_API
3636
PyAPI_FUNC(PyObject *) _PySlice_FromIndices(Py_ssize_t start, Py_ssize_t stop);
37+
PyAPI_FUNC(int) _PySlice_GetLongIndices(PySliceObject *self, PyObject *length,
38+
PyObject **start_ptr, PyObject **stop_ptr,
39+
PyObject **step_ptr);
3740
#endif
3841
PyAPI_FUNC(int) PySlice_GetIndices(PyObject *r, Py_ssize_t length,
3942
Py_ssize_t *start, Py_ssize_t *stop, Py_ssize_t *step);

‎Objects/rangeobject.c‎

Lines changed: 6 additions & 199 deletions
Original file line numberDiff line numberDiff line change
@@ -318,206 +318,18 @@ range_item(rangeobject *r, Py_ssize_t i)
318318
return res;
319319
}
320320

321-
/* Additional helpers, since the standard slice helpers
322-
* all clip to PY_SSIZE_T_MAX
323-
*/
324-
325-
/* Replace _PyEval_SliceIndex */
326-
static PyObject *
327-
compute_slice_element(PyObject *obj)
328-
{
329-
PyObject *result = NULL;
330-
if (obj != NULL) {
331-
if (PyIndex_Check(obj)) {
332-
result = PyNumber_Index(obj);
333-
}
334-
else {
335-
PyErr_SetString(PyExc_TypeError,
336-
"slice indices must be integers or "
337-
"None or have an __index__ method");
338-
}
339-
}
340-
return result;
341-
}
342-
343-
/* Replace PySlice_GetIndicesEx
344-
* Result indicates whether or not the slice is empty
345-
* (-1 = error, 0 = empty slice, 1 = slice contains elements)
346-
*/
347-
static int
348-
compute_slice_indices(rangeobject *r, PySliceObject *slice,
349-
PyObject **start, PyObject **stop, PyObject **step)
350-
{
351-
int cmp_result, has_elements;
352-
Py_ssize_t clamped_step = 0;
353-
PyObject *zero = NULL, *one = NULL, *neg_one = NULL, *candidate = NULL;
354-
PyObject *tmp_start = NULL, *tmp_stop = NULL, *tmp_step = NULL;
355-
zero = PyLong_FromLong(0);
356-
if (zero == NULL) goto Fail;
357-
one = PyLong_FromLong(1);
358-
if (one == NULL) goto Fail;
359-
neg_one = PyLong_FromLong(-1);
360-
if (neg_one == NULL) goto Fail;
361-
362-
/* Calculate step value */
363-
if (slice->step == Py_None) {
364-
clamped_step = 1;
365-
tmp_step = one;
366-
Py_INCREF(tmp_step);
367-
} else {
368-
if (!_PyEval_SliceIndex(slice->step, &clamped_step)) goto Fail;
369-
if (clamped_step == 0) {
370-
PyErr_SetString(PyExc_ValueError,
371-
"slice step cannot be zero");
372-
goto Fail;
373-
}
374-
tmp_step = compute_slice_element(slice->step);
375-
if (tmp_step == NULL) goto Fail;
376-
}
377-
378-
/* Calculate start value */
379-
if (slice->start == Py_None) {
380-
if (clamped_step < 0) {
381-
tmp_start = PyNumber_Subtract(r->length, one);
382-
if (tmp_start == NULL) goto Fail;
383-
} else {
384-
tmp_start = zero;
385-
Py_INCREF(tmp_start);
386-
}
387-
} else {
388-
candidate = compute_slice_element(slice->start);
389-
if (candidate == NULL) goto Fail;
390-
cmp_result = PyObject_RichCompareBool(candidate, zero, Py_LT);
391-
if (cmp_result == -1) goto Fail;
392-
if (cmp_result) {
393-
/* candidate < 0 */
394-
tmp_start = PyNumber_Add(r->length, candidate);
395-
if (tmp_start == NULL) goto Fail;
396-
Py_CLEAR(candidate);
397-
} else {
398-
/* candidate >= 0 */
399-
tmp_start = candidate;
400-
candidate = NULL;
401-
}
402-
cmp_result = PyObject_RichCompareBool(tmp_start, zero, Py_LT);
403-
if (cmp_result == -1) goto Fail;
404-
if (cmp_result) {
405-
/* tmp_start < 0 */
406-
Py_CLEAR(tmp_start);
407-
if (clamped_step < 0) {
408-
tmp_start = neg_one;
409-
} else {
410-
tmp_start = zero;
411-
}
412-
Py_INCREF(tmp_start);
413-
} else {
414-
/* tmp_start >= 0 */
415-
cmp_result = PyObject_RichCompareBool(tmp_start, r->length, Py_GE);
416-
if (cmp_result == -1) goto Fail;
417-
if (cmp_result) {
418-
/* tmp_start >= r->length */
419-
Py_CLEAR(tmp_start);
420-
if (clamped_step < 0) {
421-
tmp_start = PyNumber_Subtract(r->length, one);
422-
if (tmp_start == NULL) goto Fail;
423-
} else {
424-
tmp_start = r->length;
425-
Py_INCREF(tmp_start);
426-
}
427-
}
428-
}
429-
}
430-
431-
/* Calculate stop value */
432-
if (slice->stop == Py_None) {
433-
if (clamped_step < 0) {
434-
tmp_stop = neg_one;
435-
} else {
436-
tmp_stop = r->length;
437-
}
438-
Py_INCREF(tmp_stop);
439-
} else {
440-
candidate = compute_slice_element(slice->stop);
441-
if (candidate == NULL) goto Fail;
442-
cmp_result = PyObject_RichCompareBool(candidate, zero, Py_LT);
443-
if (cmp_result == -1) goto Fail;
444-
if (cmp_result) {
445-
/* candidate < 0 */
446-
tmp_stop = PyNumber_Add(r->length, candidate);
447-
if (tmp_stop == NULL) goto Fail;
448-
Py_CLEAR(candidate);
449-
} else {
450-
/* candidate >= 0 */
451-
tmp_stop = candidate;
452-
candidate = NULL;
453-
}
454-
cmp_result = PyObject_RichCompareBool(tmp_stop, zero, Py_LT);
455-
if (cmp_result == -1) goto Fail;
456-
if (cmp_result) {
457-
/* tmp_stop < 0 */
458-
Py_CLEAR(tmp_stop);
459-
if (clamped_step < 0) {
460-
tmp_stop = neg_one;
461-
} else {
462-
tmp_stop = zero;
463-
}
464-
Py_INCREF(tmp_stop);
465-
} else {
466-
/* tmp_stop >= 0 */
467-
cmp_result = PyObject_RichCompareBool(tmp_stop, r->length, Py_GE);
468-
if (cmp_result == -1) goto Fail;
469-
if (cmp_result) {
470-
/* tmp_stop >= r->length */
471-
Py_CLEAR(tmp_stop);
472-
if (clamped_step < 0) {
473-
tmp_stop = PyNumber_Subtract(r->length, one);
474-
if (tmp_stop == NULL) goto Fail;
475-
} else {
476-
tmp_stop = r->length;
477-
Py_INCREF(tmp_stop);
478-
}
479-
}
480-
}
481-
}
482-
483-
/* Check if the slice is empty or not */
484-
if (clamped_step < 0) {
485-
has_elements = PyObject_RichCompareBool(tmp_start, tmp_stop, Py_GT);
486-
} else {
487-
has_elements = PyObject_RichCompareBool(tmp_start, tmp_stop, Py_LT);
488-
}
489-
if (has_elements == -1) goto Fail;
490-
491-
*start = tmp_start;
492-
*stop = tmp_stop;
493-
*step = tmp_step;
494-
Py_DECREF(neg_one);
495-
Py_DECREF(one);
496-
Py_DECREF(zero);
497-
return has_elements;
498-
499-
Fail:
500-
Py_XDECREF(tmp_start);
501-
Py_XDECREF(tmp_stop);
502-
Py_XDECREF(tmp_step);
503-
Py_XDECREF(candidate);
504-
Py_XDECREF(neg_one);
505-
Py_XDECREF(one);
506-
Py_XDECREF(zero);
507-
return -1;
508-
}
509-
510321
static PyObject *
511322
compute_slice(rangeobject *r, PyObject *_slice)
512323
{
513324
PySliceObject *slice = (PySliceObject *) _slice;
514325
rangeobject *result;
515326
PyObject *start = NULL, *stop = NULL, *step = NULL;
516327
PyObject *substart = NULL, *substop = NULL, *substep = NULL;
517-
int has_elements;
328+
int error;
518329

519-
has_elements = compute_slice_indices(r, slice, &start, &stop, &step);
520-
if (has_elements == -1) return NULL;
330+
error = _PySlice_GetLongIndices(slice, r->length, &start, &stop, &step);
331+
if (error == -1)
332+
return NULL;
521333

522334
substep = PyNumber_Multiply(r->step, step);
523335
if (substep == NULL) goto fail;
@@ -527,13 +339,8 @@ compute_slice(rangeobject *r, PyObject *_slice)
527339
if (substart == NULL) goto fail;
528340
Py_CLEAR(start);
529341

530-
if (has_elements) {
531-
substop = compute_item(r, stop);
532-
if (substop == NULL) goto fail;
533-
} else {
534-
substop = substart;
535-
Py_INCREF(substop);
536-
}
342+
substop = compute_item(r, stop);
343+
if (substop == NULL) goto fail;
537344
Py_CLEAR(stop);
538345

539346
result = make_range_object(Py_TYPE(r), substart, substop, substep);

0 commit comments

Comments
 (0)