Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(7)

Unified Diff: Objects/sliceobject.c

Issue 14794: slice.indices raises OverflowError
Patch Set: Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« Lib/test/test_slice.py ('K') | « Lib/test/test_slice.py ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
--- 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;
storchaka 2012/11/03 21:56:32 You can move start_is_negative and stop_is_negativ
mark.dickinson 2012/11/03 23:14:48 Thanks. Will do.
- 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;
+
storchaka 2012/11/03 21:56:32 There is a suggestion. Add "start = start_as_int;
mark.dickinson 2012/11/03 23:14:48 Ah, good suggestion. I'll update the patch; than
+ 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;
storchaka 2012/11/03 21:56:32 Py_DECREF(start_as_int);
mark.dickinson 2012/11/03 23:14:48 Got it (and the other missing DECREFs below). Tha
+ 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;
storchaka 2012/11/03 21:56:32 Py_DECREF(start_as_int);
+ 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;
+
storchaka 2012/11/03 21:56:32 Same suggestion as for start_as_int.
+ 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;
storchaka 2012/11/03 21:56:32 Py_DECREF(stop_as_int);
+ 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;
storchaka 2012/11/03 21:56:32 Py_DECREF(stop_as_int);
+ 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\
« Lib/test/test_slice.py ('K') | « Lib/test/test_slice.py ('k') | no next file » | no next file with comments »

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+