diff -r f1a25c088b8f Lib/test/test_slice.py --- a/Lib/test/test_slice.py Fri Nov 02 14:49:02 2012 +0100 +++ b/Lib/test/test_slice.py Sat Nov 03 19:16:32 2012 +0000 @@ -4,8 +4,56 @@ from test import support from pickle import loads, dumps +import operator import sys + +def slice_indices(slice, length): + """ + Python reference implementation of the slice.indices method. + + """ + # Compute step and length as integers. + step = 1 if slice.step is None else operator.index(slice.step) + length = operator.index(length) + + # Check error conditions. + if step == 0: + raise ValueError("slice step cannot be zero") + if length < 0: + raise ValueError("length should be nonnegative") + + # Get lower and upper bounds for start and stop. + lower = -1 if step < 0 else 0 + upper = length - 1 if step < 0 else length + + # Compute start. + if slice.start is None: + start = upper if step < 0 else lower + else: + start = operator.index(slice.start) + start = max(start + length, lower) if start < 0 else min(start, upper) + + # Compute stop. + if slice.stop is None: + stop = lower if step < 0 else upper + else: + stop = operator.index(slice.stop) + stop = max(stop + length, lower) if stop < 0 else min(stop, upper) + + return start, stop, step + + +# Class providing an __index__ method. Used in testing. + +class MyIndexable(object): + def __init__(self, value): + self.value = value + + def __index__(self): + return self.value + + class SliceTest(unittest.TestCase): def test_constructor(self): @@ -75,6 +123,27 @@ s = slice(obj) self.assertTrue(s.stop is obj) + def check_indices(self, slice, length): + try: + expected = slice.indices(length) + except ValueError: + expected = "valueerror" + try: + actual = slice_indices(slice, length) + except ValueError: + actual = "valueerror" + + self.assertEqual(actual, expected) + + if slice.step == 0: + return + + # Check the indices we're getting. + self.assertEqual( + range(*slice_indices(slice, length)), + range(length)[slice], + ) + def test_indices(self): self.assertEqual(slice(None ).indices(10), (0, 10, 1)) self.assertEqual(slice(None, None, 2).indices(10), (0, 10, 2)) @@ -108,7 +177,42 @@ self.assertEqual(list(range(10))[::sys.maxsize - 1], [0]) - self.assertRaises(OverflowError, slice(None).indices, 1<<100) + # Check a variety of start, stop, step and length values, including + # values exceeding sys.maxsize (see issue #14794). + vals = [None, -2**100, -2**30, -53, -7, -1, 0, 1, 7, 53, 2**30, 2**100] + lengths = [0, 1, 7, 53, 2**30, 2**100] + for start in vals: + for stop in vals: + for step in vals: + s = slice(start, stop, step) + for length in lengths: + self.check_indices(s, length) + + # Negative length should raise ValueError + with self.assertRaises(ValueError): + slice(None).indices(-1) + + # Zero step should raise ValueError + with self.assertRaises(ValueError): + slice(0, 10, 0).indices(5) + + # Using a start, stop or step or length that can't be interpreted as an + # integer should give a TypeError ... + with self.assertRaises(TypeError): + slice(0.0, 10, 1).indices(5) + with self.assertRaises(TypeError): + slice(0, 10.0, 1).indices(5) + with self.assertRaises(TypeError): + slice(0, 10, 1.0).indices(5) + with self.assertRaises(TypeError): + slice(0, 10, 1).indices(5.0) + + # ... but it should be fine to use a custom class that provides index. + self.assertEqual(slice(0, 10, 1).indices(5), (0, 5, 1)) + self.assertEqual(slice(MyIndexable(0), 10, 1).indices(5), (0, 5, 1)) + self.assertEqual(slice(0, MyIndexable(10), 1).indices(5), (0, 5, 1)) + self.assertEqual(slice(0, 10, MyIndexable(1)).indices(5), (0, 5, 1)) + self.assertEqual(slice(0, 10, 1).indices(MyIndexable(5)), (0, 5, 1)) def test_setslice_without_getslice(self): tmp = [] diff -r f1a25c088b8f Objects/sliceobject.c --- a/Objects/sliceobject.c Fri Nov 02 14:49:02 2012 +0100 +++ b/Objects/sliceobject.c Sat Nov 03 19:16:32 2012 +0000 @@ -302,22 +302,179 @@ static PyObject* slice_indices(PySliceObject* self, PyObject* len) { - Py_ssize_t ilen, start, stop, step, slicelength; + /* Version that works for arbitrarily large start, stop, step and + * length. */ + /* To do: reinstate old version as an optimization for the small-int case. + (And do some timings to find out if it's even worth keeping that + optimization.) */ + /* To do: error case for negative length. */ - ilen = PyNumber_AsSsize_t(len, PyExc_OverflowError); + PyObject *start=NULL, *stop=NULL, *step=NULL; + PyObject *length=NULL, *upper=NULL, *lower=NULL, *zero=NULL; + int step_is_negative, stop_is_negative, start_is_negative, cmp; - if (ilen == -1 && PyErr_Occurred()) { + /* We'll need 0 later to determine whether start and stop are negative. */ + zero = PyLong_FromLong(0L); + if (zero == NULL) return NULL; + + /* Compute step and length as integers. */ + length = PyNumber_Index(len); + if (length == NULL) + goto error; + + if (self->step == Py_None) + step = PyLong_FromLong(1L); + else + step = PyNumber_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; + } + else if (cmp) { + PyErr_SetString(PyExc_ValueError, + "length should not be negative"); + goto error; } - if (PySlice_GetIndicesEx((PyObject*)self, ilen, &start, &stop, - &step, &slicelength) < 0) { - return NULL; + cmp = PyObject_RichCompareBool(step, zero, Py_EQ); + if (cmp < 0) { + goto error; + } + else if (cmp) { + PyErr_SetString(PyExc_ValueError, + "slice step cannot be zero"); + goto error; } - return Py_BuildValue("(nnn)", start, stop, step); + /* 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; + } + else if (step_is_negative) { + lower = PyLong_FromLong(-1L); + if (lower == NULL) + goto error; + + upper = PyNumber_Add(length, lower); + if (upper == NULL) + goto error; + } + else { + lower = zero; + Py_INCREF(lower); + upper = length; + Py_INCREF(upper); + } + + /* Compute start. */ + if (self->start == Py_None) { + start = step_is_negative ? upper : lower; + Py_INCREF(start); + } + else { + PyObject *start_as_int = PyNumber_Index(self->start); + if (start_as_int == NULL) + goto error; + + start_is_negative = PyObject_RichCompareBool(start_as_int, zero, Py_LT); + if (start_is_negative < 0) { + Py_DECREF(start_as_int); + goto error; + } + else if (start_is_negative) { + PyObject *tmp = PyNumber_Add(start_as_int, length); + Py_DECREF(start_as_int); + start_as_int = tmp; + if (start_as_int == NULL) + goto error; + + cmp = PyObject_RichCompareBool(start_as_int, lower, Py_LE); + if (cmp < 0) + goto error; + else if (cmp) + start = lower; + else + start = start_as_int; + } + else { + cmp = PyObject_RichCompareBool(start_as_int, upper, Py_GE); + if (cmp < 0) + goto error; + else if (cmp) + start = upper; + else + start = start_as_int; + } + Py_INCREF(start); + Py_DECREF(start_as_int); + } + + /* Compute stop. */ + if (self->stop == Py_None) { + stop = step_is_negative ? lower : upper; + Py_INCREF(stop); + } + else { + PyObject *stop_as_int = PyNumber_Index(self->stop); + if (stop_as_int == NULL) + goto error; + + stop_is_negative = PyObject_RichCompareBool(stop_as_int, zero, Py_LT); + if (stop_is_negative < 0) { + Py_DECREF(stop_as_int); + goto error; + } + else if (stop_is_negative) { + PyObject *tmp = PyNumber_Add(stop_as_int, length); + Py_DECREF(stop_as_int); + stop_as_int = tmp; + if (stop_as_int == NULL) + goto error; + + cmp = PyObject_RichCompareBool(stop_as_int, lower, Py_LE); + if (cmp < 0) + goto error; + else if (cmp) + stop = lower; + else + stop = stop_as_int; + } + else { + cmp = PyObject_RichCompareBool(stop_as_int, upper, Py_GE); + if (cmp < 0) + goto error; + else if (cmp) + stop = upper; + else + stop = stop_as_int; + } + Py_INCREF(stop); + Py_DECREF(stop_as_int); + } + Py_DECREF(upper); + Py_DECREF(lower); + Py_DECREF(length); + Py_DECREF(zero); + return Py_BuildValue("(NNN)", start, stop, step); + + error: + Py_XDECREF(start); + Py_XDECREF(stop); + Py_XDECREF(step); + Py_XDECREF(upper); + Py_XDECREF(lower); + Py_XDECREF(length); + Py_XDECREF(zero); + return NULL; } + PyDoc_STRVAR(slice_indices_doc, "S.indices(len) -> (start, stop, stride)\n\ \n\