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

Delta Between Two Patch Sets: Objects/sliceobject.c

Issue 14794: slice.indices raises OverflowError
Left Patch Set: Created 6 years, 9 months ago
Right Patch Set: Created 6 years, 9 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « Lib/test/test_slice.py ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 Written by Jim Hugunin and Chris Chase. 2 Written by Jim Hugunin and Chris Chase.
3 3
4 This includes both the singular ellipsis object and slice objects. 4 This includes both the singular ellipsis object and slice objects.
5 5
6 Guido, feel free to do whatever you want in the way of copyrights 6 Guido, feel free to do whatever you want in the way of copyrights
7 for this file. 7 for this file.
8 */ 8 */
9 9
10 /* 10 /*
(...skipping 281 matching lines...) Expand 10 before | Expand all | Expand 10 after
292 return PyUnicode_FromFormat("slice(%R, %R, %R)", r->start, r->stop, r->step) ; 292 return PyUnicode_FromFormat("slice(%R, %R, %R)", r->start, r->stop, r->step) ;
293 } 293 }
294 294
295 static PyMemberDef slice_members[] = { 295 static PyMemberDef slice_members[] = {
296 {"start", T_OBJECT, offsetof(PySliceObject, start), READONLY}, 296 {"start", T_OBJECT, offsetof(PySliceObject, start), READONLY},
297 {"stop", T_OBJECT, offsetof(PySliceObject, stop), READONLY}, 297 {"stop", T_OBJECT, offsetof(PySliceObject, stop), READONLY},
298 {"step", T_OBJECT, offsetof(PySliceObject, step), READONLY}, 298 {"step", T_OBJECT, offsetof(PySliceObject, step), READONLY},
299 {0} 299 {0}
300 }; 300 };
301 301
302 /* Helper function to convert a slice argument to a PyLong, and raise TypeError
303 with a suitable message on failure. */
304
305 static PyObject*
306 evaluate_slice_index(PyObject *v)
307 {
308 if (PyIndex_Check(v)) {
storchaka 2012/11/04 12:29:34 Why not EAFP? PyObject *index = PyNumber_Index(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 }
318
319 /* Implementation of slice.indices. */
320
302 static PyObject* 321 static PyObject*
303 slice_indices(PySliceObject* self, PyObject* len) 322 slice_indices(PySliceObject* self, PyObject* len)
304 { 323 {
305 /* Version that works for arbitrarily large start, stop, step and
306 * length. */
307 /* To do: reinstate old version as an optimization for the small-int case.
308 (And do some timings to find out if it's even worth keeping that
309 optimization.) */
310 /* To do: error case for negative length. */
311
312 PyObject *start=NULL, *stop=NULL, *step=NULL; 324 PyObject *start=NULL, *stop=NULL, *step=NULL;
313 PyObject *length=NULL, *upper=NULL, *lower=NULL, *zero=NULL; 325 PyObject *length=NULL, *upper=NULL, *lower=NULL, *zero=NULL;
314 int step_is_negative, stop_is_negative, start_is_negative, cmp; 326 int step_is_negative, cmp;
315 327
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.
316 /* We'll need 0 later to determine whether start and stop are negative. */
317 zero = PyLong_FromLong(0L); 328 zero = PyLong_FromLong(0L);
318 if (zero == NULL) 329 if (zero == NULL)
319 return NULL; 330 return NULL;
320 331
321 /* Compute step and length as integers. */ 332 /* Compute step and length as integers. */
322 length = PyNumber_Index(len); 333 length = PyNumber_Index(len);
323 if (length == NULL) 334 if (length == NULL)
324 goto error; 335 goto error;
325 336
326 if (self->step == Py_None) 337 if (self->step == Py_None)
327 step = PyLong_FromLong(1L); 338 step = PyLong_FromLong(1L);
328 else 339 else
329 step = PyNumber_Index(self->step); 340 step = evaluate_slice_index(self->step);
330 if (step == NULL) 341 if (step == NULL)
331 goto error; 342 goto error;
332 343
333 /* Raise ValueError for negative length or zero step. */ 344 /* Raise ValueError for negative length or zero step. */
334 cmp = PyObject_RichCompareBool(length, zero, Py_LT); 345 cmp = PyObject_RichCompareBool(length, zero, Py_LT);
335 if (cmp < 0) { 346 if (cmp < 0) {
336 goto error; 347 goto error;
337 } 348 }
338 else if (cmp) { 349 if (cmp) {
339 PyErr_SetString(PyExc_ValueError, 350 PyErr_SetString(PyExc_ValueError,
340 "length should not be negative"); 351 "length should not be negative");
341 goto error; 352 goto error;
342 } 353 }
343 354
344 cmp = PyObject_RichCompareBool(step, zero, Py_EQ); 355 cmp = PyObject_RichCompareBool(step, zero, Py_EQ);
345 if (cmp < 0) { 356 if (cmp < 0) {
346 goto error; 357 goto error;
347 } 358 }
348 else if (cmp) { 359 if (cmp) {
349 PyErr_SetString(PyExc_ValueError, 360 PyErr_SetString(PyExc_ValueError,
350 "slice step cannot be zero"); 361 "slice step cannot be zero");
351 goto error; 362 goto error;
352 } 363 }
353 364
354 /* Find lower and upper bounds for start and stop. */ 365 /* Find lower and upper bounds for start and stop. */
355 step_is_negative = PyObject_RichCompareBool(step, zero, Py_LT); 366 step_is_negative = PyObject_RichCompareBool(step, zero, Py_LT);
356 if (step_is_negative < 0) { 367 if (step_is_negative < 0) {
357 goto error; 368 goto error;
358 } 369 }
359 else if (step_is_negative) { 370 if (step_is_negative) {
360 lower = PyLong_FromLong(-1L); 371 lower = PyLong_FromLong(-1L);
361 if (lower == NULL) 372 if (lower == NULL)
362 goto error; 373 goto error;
363 374
364 upper = PyNumber_Add(length, lower); 375 upper = PyNumber_Add(length, lower);
365 if (upper == NULL) 376 if (upper == NULL)
366 goto error; 377 goto error;
367 } 378 }
368 else { 379 else {
369 lower = zero; 380 lower = zero;
370 Py_INCREF(lower); 381 Py_INCREF(lower);
371 upper = length; 382 upper = length;
372 Py_INCREF(upper); 383 Py_INCREF(upper);
373 } 384 }
374 385
375 /* Compute start. */ 386 /* Compute start. */
376 if (self->start == Py_None) { 387 if (self->start == Py_None) {
377 start = step_is_negative ? upper : lower; 388 start = step_is_negative ? upper : lower;
378 Py_INCREF(start); 389 Py_INCREF(start);
379 } 390 }
380 else { 391 else {
381 PyObject *start_as_int = PyNumber_Index(self->start); 392 start = evaluate_slice_index(self->start);
382 if (start_as_int == NULL) 393 if (start == NULL)
383 goto error; 394 goto error;
384 395
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
385 start_is_negative = PyObject_RichCompareBool(start_as_int, zero, Py_LT); 396 cmp = PyObject_RichCompareBool(start, zero, Py_LT);
386 if (start_is_negative < 0) { 397 if (cmp < 0)
387 Py_DECREF(start_as_int);
388 goto error; 398 goto error;
389 } 399 if (cmp) {
390 else if (start_is_negative) { 400 /* start += length */
391 PyObject *tmp = PyNumber_Add(start_as_int, length); 401 PyObject *tmp = PyNumber_Add(start, length);
392 Py_DECREF(start_as_int); 402 Py_DECREF(start);
393 start_as_int = tmp; 403 start = tmp;
394 if (start_as_int == NULL) 404 if (start == NULL)
395 goto error; 405 goto error;
396 406
397 cmp = PyObject_RichCompareBool(start_as_int, lower, Py_LE); 407 cmp = PyObject_RichCompareBool(start, lower, Py_LT);
398 if (cmp < 0) 408 if (cmp < 0)
399 goto error; 409 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
400 else if (cmp) 410 if (cmp) {
411 Py_INCREF(lower);
412 Py_DECREF(start);
401 start = lower; 413 start = lower;
402 else 414 }
403 start = start_as_int;
404 } 415 }
405 else { 416 else {
406 cmp = PyObject_RichCompareBool(start_as_int, upper, Py_GE); 417 cmp = PyObject_RichCompareBool(start, upper, Py_GT);
407 if (cmp < 0) 418 if (cmp < 0)
408 goto error; 419 goto error;
storchaka 2012/11/03 21:56:32 Py_DECREF(start_as_int);
409 else if (cmp) 420 if (cmp) {
421 Py_INCREF(upper);
422 Py_DECREF(start);
410 start = upper; 423 start = upper;
411 else 424 }
412 start = start_as_int;
413 } 425 }
414 Py_INCREF(start);
415 Py_DECREF(start_as_int);
416 } 426 }
417 427
418 /* Compute stop. */ 428 /* Compute stop. */
419 if (self->stop == Py_None) { 429 if (self->stop == Py_None) {
420 stop = step_is_negative ? lower : upper; 430 stop = step_is_negative ? lower : upper;
421 Py_INCREF(stop); 431 Py_INCREF(stop);
422 } 432 }
423 else { 433 else {
424 PyObject *stop_as_int = PyNumber_Index(self->stop); 434 stop = evaluate_slice_index(self->stop);
425 if (stop_as_int == NULL) 435 if (stop == NULL)
426 goto error; 436 goto error;
427 437
storchaka 2012/11/03 21:56:32 Same suggestion as for start_as_int.
428 stop_is_negative = PyObject_RichCompareBool(stop_as_int, zero, Py_LT); 438 cmp = PyObject_RichCompareBool(stop, zero, Py_LT);
429 if (stop_is_negative < 0) { 439 if (cmp < 0)
430 Py_DECREF(stop_as_int);
431 goto error; 440 goto error;
432 } 441 if (cmp) {
433 else if (stop_is_negative) { 442 /* stop += length */
434 PyObject *tmp = PyNumber_Add(stop_as_int, length); 443 PyObject *tmp = PyNumber_Add(stop, length);
435 Py_DECREF(stop_as_int); 444 Py_DECREF(stop);
436 stop_as_int = tmp; 445 stop = tmp;
437 if (stop_as_int == NULL) 446 if (stop == NULL)
438 goto error; 447 goto error;
439 448
440 cmp = PyObject_RichCompareBool(stop_as_int, lower, Py_LE); 449 cmp = PyObject_RichCompareBool(stop, lower, Py_LT);
441 if (cmp < 0) 450 if (cmp < 0)
442 goto error; 451 goto error;
storchaka 2012/11/03 21:56:32 Py_DECREF(stop_as_int);
443 else if (cmp) 452 if (cmp) {
453 Py_INCREF(lower);
454 Py_DECREF(stop);
444 stop = lower; 455 stop = lower;
445 else 456 }
446 stop = stop_as_int;
447 } 457 }
448 else { 458 else {
449 cmp = PyObject_RichCompareBool(stop_as_int, upper, Py_GE); 459 cmp = PyObject_RichCompareBool(stop, upper, Py_GT);
450 if (cmp < 0) 460 if (cmp < 0)
451 goto error; 461 goto error;
storchaka 2012/11/03 21:56:32 Py_DECREF(stop_as_int);
452 else if (cmp) 462 if (cmp) {
463 Py_INCREF(upper);
464 Py_DECREF(stop);
453 stop = upper; 465 stop = upper;
454 else 466 }
455 stop = stop_as_int;
456 } 467 }
457 Py_INCREF(stop); 468 }
458 Py_DECREF(stop_as_int); 469
459 }
460 Py_DECREF(upper); 470 Py_DECREF(upper);
461 Py_DECREF(lower); 471 Py_DECREF(lower);
462 Py_DECREF(length); 472 Py_DECREF(length);
463 Py_DECREF(zero); 473 Py_DECREF(zero);
464 return Py_BuildValue("(NNN)", start, stop, step); 474 return Py_BuildValue("(NNN)", start, stop, step);
465 475
466 error: 476 error:
467 Py_XDECREF(start); 477 Py_XDECREF(start);
468 Py_XDECREF(stop); 478 Py_XDECREF(stop);
469 Py_XDECREF(step); 479 Py_XDECREF(step);
(...skipping 120 matching lines...) Expand 10 before | Expand all | Expand 10 after
590 0, /* tp_getset */ 600 0, /* tp_getset */
591 0, /* tp_base */ 601 0, /* tp_base */
592 0, /* tp_dict */ 602 0, /* tp_dict */
593 0, /* tp_descr_get */ 603 0, /* tp_descr_get */
594 0, /* tp_descr_set */ 604 0, /* tp_descr_set */
595 0, /* tp_dictoffset */ 605 0, /* tp_dictoffset */
596 0, /* tp_init */ 606 0, /* tp_init */
597 0, /* tp_alloc */ 607 0, /* tp_alloc */
598 slice_new, /* tp_new */ 608 slice_new, /* tp_new */
599 }; 609 };
LEFTRIGHT

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