Skip to content

Commit c8a6967

Browse files
mdickinsonmdickinson
committed
Issue #14794: slice.indices no longer returns OverflowError for out-of-range start, stop, step or length.
1 parent ff3d515 commit c8a6967

3 files changed

Lines changed: 291 additions & 9 deletions

File tree

‎Lib/test/test_slice.py‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,70 @@
44
from test import support
55
from pickle import loads, dumps
66

7+
import itertools
8+
import operator
79
import sys
810

11+
12+
def evaluate_slice_index(arg):
13+
"""
14+
Helper function to convert a slice argument to an integer, and raise
15+
TypeError with a suitable message on failure.
16+
17+
"""
18+
if hasattr(arg, '__index__'):
19+
return operator.index(arg)
20+
else:
21+
raise TypeError(
22+
"slice indices must be integers or "
23+
"None or have an __index__ method")
24+
25+
def slice_indices(slice, length):
26+
"""
27+
Reference implementation for the slice.indices method.
28+
29+
"""
30+
# Compute step and length as integers.
31+
length = operator.index(length)
32+
step = 1 if slice.step is None else evaluate_slice_index(slice.step)
33+
34+
# Raise ValueError for negative length or zero step.
35+
if length < 0:
36+
raise ValueError("length should not be negative")
37+
if step == 0:
38+
raise ValueError("slice step cannot be zero")
39+
40+
# Find lower and upper bounds for start and stop.
41+
lower = -1 if step < 0 else 0
42+
upper = length - 1 if step < 0 else length
43+
44+
# Compute start.
45+
if slice.start is None:
46+
start = upper if step < 0 else lower
47+
else:
48+
start = evaluate_slice_index(slice.start)
49+
start = max(start + length, lower) if start < 0 else min(start, upper)
50+
51+
# Compute stop.
52+
if slice.stop is None:
53+
stop = lower if step < 0 else upper
54+
else:
55+
stop = evaluate_slice_index(slice.stop)
56+
stop = max(stop + length, lower) if stop < 0 else min(stop, upper)
57+
58+
return start, stop, step
59+
60+
61+
# Class providing an __index__ method. Used for testing slice.indices.
62+
63+
class MyIndexable(object):
64+
def __init__(self, value):
65+
self.value = value
66+
67+
def __index__(self):
68+
return self.value
69+
70+
971
class SliceTest(unittest.TestCase):
1072

1173
def test_constructor(self):
@@ -75,6 +137,22 @@ class AnyClass:
75137
s = slice(obj)
76138
self.assertTrue(s.stop is obj)
77139

140+
def check_indices(self, slice, length):
141+
try:
142+
actual = slice.indices(length)
143+
except ValueError:
144+
actual = "valueerror"
145+
try:
146+
expected = slice_indices(slice, length)
147+
except ValueError:
148+
expected = "valueerror"
149+
self.assertEqual(actual, expected)
150+
151+
if length >= 0 and slice.step != 0:
152+
actual = range(*slice.indices(length))
153+
expected = range(length)[slice]
154+
self.assertEqual(actual, expected)
155+
78156
def test_indices(self):
79157
self.assertEqual(slice(None ).indices(10), (0, 10, 1))
80158
self.assertEqual(slice(None, None, 2).indices(10), (0, 10, 2))
@@ -108,7 +186,41 @@ def test_indices(self):
108186

109187
self.assertEqual(list(range(10))[::sys.maxsize - 1], [0])
110188

111-
self.assertRaises(OverflowError, slice(None).indices, 1<<100)
189+
# Check a variety of start, stop, step and length values, including
190+
# values exceeding sys.maxsize (see issue #14794).
191+
vals = [None, -2**100, -2**30, -53, -7, -1, 0, 1, 7, 53, 2**30, 2**100]
192+
lengths = [0, 1, 7, 53, 2**30, 2**100]
193+
for slice_args in itertools.product(vals, repeat=3):
194+
s = slice(*slice_args)
195+
for length in lengths:
196+
self.check_indices(s, length)
197+
self.check_indices(slice(0, 10, 1), -3)
198+
199+
# Negative length should raise ValueError
200+
with self.assertRaises(ValueError):
201+
slice(None).indices(-1)
202+
203+
# Zero step should raise ValueError
204+
with self.assertRaises(ValueError):
205+
slice(0, 10, 0).indices(5)
206+
207+
# Using a start, stop or step or length that can't be interpreted as an
208+
# integer should give a TypeError ...
209+
with self.assertRaises(TypeError):
210+
slice(0.0, 10, 1).indices(5)
211+
with self.assertRaises(TypeError):
212+
slice(0, 10.0, 1).indices(5)
213+
with self.assertRaises(TypeError):
214+
slice(0, 10, 1.0).indices(5)
215+
with self.assertRaises(TypeError):
216+
slice(0, 10, 1).indices(5.0)
217+
218+
# ... but it should be fine to use a custom class that provides index.
219+
self.assertEqual(slice(0, 10, 1).indices(5), (0, 5, 1))
220+
self.assertEqual(slice(MyIndexable(0), 10, 1).indices(5), (0, 5, 1))
221+
self.assertEqual(slice(0, MyIndexable(10), 1).indices(5), (0, 5, 1))
222+
self.assertEqual(slice(0, 10, MyIndexable(1)).indices(5), (0, 5, 1))
223+
self.assertEqual(slice(0, 10, 1).indices(MyIndexable(5)), (0, 5, 1))
112224

113225
def test_setslice_without_getslice(self):
114226
tmp = []

‎Misc/NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ What's New in Python 3.4.0 Alpha 1?
1010
Core and Builtins
1111
-----------------
1212

13+
- Issue #14794: Fix slice.indices to return correct results for huge values,
14+
rather than raising OverflowError.
15+
1316
- Issue #15001: fix segfault on "del sys.module['__main__']". Patch by Victor
1417
Stinner.
1518

‎Objects/sliceobject.c‎

Lines changed: 175 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -299,25 +299,192 @@ static PyMemberDef slice_members[] = {
299299
{0}
300300
};
301301

302+
/* Helper function to convert a slice argument to a PyLong, and raise TypeError
303+
with a suitable message on failure. */
304+
302305
static PyObject*
303-
slice_indices(PySliceObject* self, PyObject* len)
306+
evaluate_slice_index(PyObject *v)
304307
{
305-
Py_ssize_t ilen, start, stop, step, slicelength;
308+
if (PyIndex_Check(v)) {
309+
return PyNumber_Index(v);
310+
}
311+
else {
312+
PyErr_SetString(PyExc_TypeError,
313+
"slice indices must be integers or "
314+
"None or have an __index__ method");
315+
return NULL;
316+
}
317+
}
306318

307-
ilen = PyNumber_AsSsize_t(len, PyExc_OverflowError);
319+
/* Implementation of slice.indices. */
320+
321+
static PyObject*
322+
slice_indices(PySliceObject* self, PyObject* len)
323+
{
324+
PyObject *start=NULL, *stop=NULL, *step=NULL;
325+
PyObject *length=NULL, *upper=NULL, *lower=NULL, *zero=NULL;
326+
int step_is_negative, cmp;
308327

309-
if (ilen == -1 && PyErr_Occurred()) {
328+
zero = PyLong_FromLong(0L);
329+
if (zero == NULL)
310330
return NULL;
331+
332+
/* Compute step and length as integers. */
333+
length = PyNumber_Index(len);
334+
if (length == NULL)
335+
goto error;
336+
337+
if (self->step == Py_None)
338+
step = PyLong_FromLong(1L);
339+
else
340+
step = evaluate_slice_index(self->step);
341+
if (step == NULL)
342+
goto error;
343+
344+
/* Raise ValueError for negative length or zero step. */
345+
cmp = PyObject_RichCompareBool(length, zero, Py_LT);
346+
if (cmp < 0) {
347+
goto error;
348+
}
349+
if (cmp) {
350+
PyErr_SetString(PyExc_ValueError,
351+
"length should not be negative");
352+
goto error;
311353
}
312354

313-
if (PySlice_GetIndicesEx((PyObject*)self, ilen, &start, &stop,
314-
&step, &slicelength) < 0) {
315-
return NULL;
355+
cmp = PyObject_RichCompareBool(step, zero, Py_EQ);
356+
if (cmp < 0) {
357+
goto error;
358+
}
359+
if (cmp) {
360+
PyErr_SetString(PyExc_ValueError,
361+
"slice step cannot be zero");
362+
goto error;
316363
}
317364

318-
return Py_BuildValue("(nnn)", start, stop, step);
365+
/* Find lower and upper bounds for start and stop. */
366+
step_is_negative = PyObject_RichCompareBool(step, zero, Py_LT);
367+
if (step_is_negative < 0) {
368+
goto error;
369+
}
370+
if (step_is_negative) {
371+
lower = PyLong_FromLong(-1L);
372+
if (lower == NULL)
373+
goto error;
374+
375+
upper = PyNumber_Add(length, lower);
376+
if (upper == NULL)
377+
goto error;
378+
}
379+
else {
380+
lower = zero;
381+
Py_INCREF(lower);
382+
upper = length;
383+
Py_INCREF(upper);
384+
}
385+
386+
/* Compute start. */
387+
if (self->start == Py_None) {
388+
start = step_is_negative ? upper : lower;
389+
Py_INCREF(start);
390+
}
391+
else {
392+
start = evaluate_slice_index(self->start);
393+
if (start == NULL)
394+
goto error;
395+
396+
cmp = PyObject_RichCompareBool(start, zero, Py_LT);
397+
if (cmp < 0)
398+
goto error;
399+
if (cmp) {
400+
/* start += length */
401+
PyObject *tmp = PyNumber_Add(start, length);
402+
Py_DECREF(start);
403+
start = tmp;
404+
if (start == NULL)
405+
goto error;
406+
407+
cmp = PyObject_RichCompareBool(start, lower, Py_LT);
408+
if (cmp < 0)
409+
goto error;
410+
if (cmp) {
411+
Py_INCREF(lower);
412+
Py_DECREF(start);
413+
start = lower;
414+
}
415+
}
416+
else {
417+
cmp = PyObject_RichCompareBool(start, upper, Py_GT);
418+
if (cmp < 0)
419+
goto error;
420+
if (cmp) {
421+
Py_INCREF(upper);
422+
Py_DECREF(start);
423+
start = upper;
424+
}
425+
}
426+
}
427+
428+
/* Compute stop. */
429+
if (self->stop == Py_None) {
430+
stop = step_is_negative ? lower : upper;
431+
Py_INCREF(stop);
432+
}
433+
else {
434+
stop = evaluate_slice_index(self->stop);
435+
if (stop == NULL)
436+
goto error;
437+
438+
cmp = PyObject_RichCompareBool(stop, zero, Py_LT);
439+
if (cmp < 0)
440+
goto error;
441+
if (cmp) {
442+
/* stop += length */
443+
PyObject *tmp = PyNumber_Add(stop, length);
444+
Py_DECREF(stop);
445+
stop = tmp;
446+
if (stop == NULL)
447+
goto error;
448+
449+
cmp = PyObject_RichCompareBool(stop, lower, Py_LT);
450+
if (cmp < 0)
451+
goto error;
452+
if (cmp) {
453+
Py_INCREF(lower);
454+
Py_DECREF(stop);
455+
stop = lower;
456+
}
457+
}
458+
else {
459+
cmp = PyObject_RichCompareBool(stop, upper, Py_GT);
460+
if (cmp < 0)
461+
goto error;
462+
if (cmp) {
463+
Py_INCREF(upper);
464+
Py_DECREF(stop);
465+
stop = upper;
466+
}
467+
}
468+
}
469+
470+
Py_DECREF(upper);
471+
Py_DECREF(lower);
472+
Py_DECREF(length);
473+
Py_DECREF(zero);
474+
return Py_BuildValue("(NNN)", start, stop, step);
475+
476+
error:
477+
Py_XDECREF(start);
478+
Py_XDECREF(stop);
479+
Py_XDECREF(step);
480+
Py_XDECREF(upper);
481+
Py_XDECREF(lower);
482+
Py_XDECREF(length);
483+
Py_XDECREF(zero);
484+
return NULL;
319485
}
320486

487+
321488
PyDoc_STRVAR(slice_indices_doc,
322489
"S.indices(len) -> (start, stop, stride)\n\
323490
\n\

0 commit comments

Comments
 (0)