Patch attached (index_overflow.diff) that causes
__index__() to raise OverflowError for really big
numbers instead of silently clipping them.

The approach in the patch is a "minimal fix" that is as
ugly as hell (3 different error return codes!), so I'm
going to try for a cleaner version that changes
nb_index to return a PyObject* (then the client code
can decide whether to convert to Py_ssize_t or not, and
whether to clip or raise OverflowError when doing so).

Logged In: YES 
user_id=31435

Since I don't think this is a sane approach, I'm not going
to spend time reviewing it :-)  Suggest working out what's
/wanted/ on python-dev first, including beefing up PEP 357
so it spells out the revised intents.

Logged In: YES 
user_id=1038590

Attaching the patch that approaches the problem from the
ground up by redesigning the PEP 357 C API to meet the needs
of the actual use cases in the standard library.

Logged In: YES 
user_id=1038590

Revised the patch to further reduce code duplication in the
implementation, and to reduce the divergence from PEP 357.

The functions in the C API now have the names PyNumber_Index
(raises IndexError), PyNumber_AsSsize_t (raises
OverflowError) and PyNumber_AsClippedSsize_t (clamps to
PY_SSIZE_T_MIN/MAX), and are no longer exposed through the
operator module. Python code should either use __index__()
directly (if not constrained to concrete containers) or else
use operator.index().

Logged In: YES 
user_id=33168

The description is no longer valid, this is not a minimal patch.

I haven't thought about the actual design of the patch, but
I have a bunch of issues.  In no particular, order:
 * tests should not use the assert statement, but self.assert*
 * There are a bunch of formatting changes (one place with a
couple of extra blank lines, other places where tabs became
spaces or vica versa)
 * I think there might be behaviour changes wrt checking for
sq_ass_item and sq_get_item and raising type_error()s.  I
didn't think about this hard, but just from a quick review
there was a red flag.
 * PyNumber_Index(o, is_index) is a bogus naming.  Either
the API should change or is_index should change.  It doesn't
make sense to me that you would call an API to get the index
that is not an index.
 * int_index became int_int, same with long, but not in
other places.  I don't understand the name change.

I need to think more about the structure of this patch. 
There's something I don't like, but I'm not sure what it is
without further review.

More eyes would definitely help.

Logged In: YES 
user_id=1038590

The minimal fix is still attached (the index_overflow one).
That's the patch Tim didn't like. The latter half of the
description covers the pep357-fix patch versions.

The API's in the more comprehensive patches (_Index,
_AsSsize_t, _AsClippedSsize_t) were based on minimising the
boilerplate associated with actually *using* those functions
to implement the existing operations in the standard library.

Without the is_index flag, the mp_subscript implementations
all need to perform their own check for whether or not the
object provides __index__ or not, since they don't want to
raise a TypeError unless the object is *also* not a slice. .

However, I'm open to inverting the sense of is_index and
changing the name of the output variable to typeerror (an
early version of the code actually had it that way around).
The only impact on the patch is a bunch of name changes to
different variables.

In terms of error messages, the patch definitely changes
some of the exact wording of raised exceptions. It doesn't
change the types, though.

The specific change to PyObject_GetItem and friends is that
the error message becomes "TypeError: 'foo' object is
unsubscriptable" instead of the old "TypeError: 'foo' object
is unindexable". The old error message can be restored if
you prefer by moving the type check back after the check for
whether or not the key is a valid index. However, it really
didn't make a lot of sense to me to check the key before
we'd even confirmed that the object actually supported
subscripting.

For the unit tests, I eliminated the use of assert
statements in v3 of the patch (as well as updating the tests
to cover a bunch of problems with the v2 patch that Travis
noted, and to avoid running the same tests multiple times).

int_index and long_index went away because the signature
change to the nb_index slot (returning PyObject *) meant
that the existing int_int and long_long functions were
sufficient - there was no need to write new functions
specifically for the nb_index slot. This actually applies to
any extension type that implements nb_index - it should be
populated with the same value as either their nb_int slot or
their nb_long slot (depending on whether or not the value
will always fit in a Python int)

In terms of style (and what may be bothering you), I'm not
aware of any existing public CPython API that supports the
use of an output flag to indicate errors instead of
requiring that callers check PyErr_Occurred. The
straightforward approach of providing a PyNumber_IndexCheck
API is certainly feasible (and has an identical line count
in the mp_subscript functions), but comes at the cost of
doing the check twice in every mp_subscript function (once
directly and once inside PyNumber_Index).

The old code didn't have this problem with doing the same
check in two different places because it was merrily calling
the nb_index slot directly all over the place instead of
going through the abstract API.

Logged In: YES 
user_id=1038590

Added version 4 of the patch. Changes:
  - is_index is now type_err, with the logical sense
inverted and corresponding changes made to other parts of
the patch (I didn't use type_error because that would have
caused a name clash inside abstract.c)
  - for consistency in terminology, the internal function
used for long conversion is now _PyLong_AsClippedSsize_t to
match the PyNumber function, and the output variable used to
report clipping is named clipped
  - an error message from _PyEval_SliceIndex had changed
gratuitously, so I restored the original message.

Logged In: YES 
user_id=1038590

Put the priority back to 9 (I accidentally changed it back
to 7 when I attached the new patch)

Logged In: YES 
user_id=1038590

Version 5 fast tracks builtin longs as well as ints

Logged In: YES 
user_id=5296

I'm attaching a patch that fixes the problem using a more
traditional C-API.  

Their are still 3 new C-API calls --- 1 is actually a macro 

* PyIndex_Check(obj)

* PyNumber_Index(obj)  ---  is to nb_index as PyNumber_Int
is to nb_int

* PyNumber_AsSsize_t(obj, err)  --- the second argument
allows specifying the error when conversion to Pyssize_t
raises an Overflow.  If err == NULL, the value is clipped
instead of raising any error.  OverflowError and IndexError
are the two errors used in Python itself and the standard
library. 

* I kept the same unit-tests from Nick except that
subclasses for integers are allowed as slice indices as this
does not cause recursion. 

Logged In: YES 
user_id=1038590

I'm withdrawing this in favour of Travis's patch #1538606 -
it's much more in line with the idioms used in other parts
of the C API. It isn't worth messing up the API just to
avoid checking for the existing of the nb_index slot twice.

With Travis's patch, it's still straightforward for
extension modules to do their own overflow handling, too -
call PyNumber_AsSsize_t, and if an overflow error gets
triggered, call PyNumber_Index to find out whether it was
positive or negative overflow.