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

#14794: slice.indices raises OverflowError

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 10 months ago by paulup
Modified:
6 years, 10 months ago
Reviewers:
storchaka, dickinsm
CC:
mark.dickinson, ned.deily, eric.araujo, devnull_psf.upfronthosting.co.za, hynek, paulup_gmail.com, storchaka
Visibility:
Public.

Patch Set 1 #

Total comments: 12

Patch Set 2 #

Patch Set 3 #

Total comments: 1

Patch Set 4 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_slice.py View 1 2 3 3 chunks +113 lines, -1 line 0 comments Download
Objects/sliceobject.c View 1 2 3 1 chunk +175 lines, -8 lines 1 comment Download

Messages

Total messages: 8
storchaka_gmail.com
http://bugs.python.org/review/14794/diff/6492/Lib/test/test_slice.py File Lib/test/test_slice.py (right): http://bugs.python.org/review/14794/diff/6492/Lib/test/test_slice.py#newcode11 Lib/test/test_slice.py:11: def slice_indices(slice, length): Can we use Python implementation for ...
6 years, 10 months ago #1
mark.dickinson
http://bugs.python.org/review/14794/diff/6492/Lib/test/test_slice.py File Lib/test/test_slice.py (right): http://bugs.python.org/review/14794/diff/6492/Lib/test/test_slice.py#newcode11 Lib/test/test_slice.py:11: def slice_indices(slice, length): I don't understand what you mean. ...
6 years, 10 months ago #2
storchaka_gmail.com
http://bugs.python.org/review/14794/diff/6499/Objects/sliceobject.c File Objects/sliceobject.c (right): http://bugs.python.org/review/14794/diff/6499/Objects/sliceobject.c#newcode398 Objects/sliceobject.c:398: cmp = PyObject_RichCompareBool(start, upper, Py_GE); This is only bikeshedding, ...
6 years, 10 months ago #3
mark.dickinson
On 2012/11/04 10:55:06, storchaka wrote: > This is only bikeshedding, but I prefer Py_GT here. ...
6 years, 10 months ago #4
mark.dickinson
On 2012/11/04 11:11:49, mark.dickinson wrote: > On 2012/11/04 10:55:06, storchaka wrote: > > This is ...
6 years, 10 months ago #5
storchaka_gmail.com
http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c File Objects/sliceobject.c (right): http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c#newcode308 Objects/sliceobject.c:308: if (PyIndex_Check(v)) { Why not EAFP? PyObject *index = ...
6 years, 10 months ago #6
mark.dickinson
On 2012/11/04 12:29:34, storchaka wrote: > http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c > File Objects/sliceobject.c (right): > > http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c#newcode308 > ...
6 years, 10 months ago #7
storchaka_gmail.com
6 years, 10 months ago #8
On 2012/11/04 12:56:39, mark.dickinson wrote:
> On 2012/11/04 12:29:34, storchaka wrote:
> > http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c
> > File Objects/sliceobject.c (right):
> > 
> >
http://bugs.python.org/review/14794/diff/6504/Objects/sliceobject.c#newcode308
> > Objects/sliceobject.c:308: if (PyIndex_Check(v)) {
> > Why not EAFP?
> > 
> > PyObject *index = PyNumber_Index(v);
> > if (index == NULL) {
> >     PyErr_SetString(...
> 
> Because if the PyNumber_Index fails due to __index__ raising an exception,
that
> exception should be propagated, whereas if it fails due to not having an
> __index__ method, we should be explicitly raising the TypeError.  We could do
an
> extra PyErr_Occurred check, but I don't think that really makes it any simpler
> than the current code.

Well, I see now.
Sign in to reply to this message.

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