[Patches] [ python-Patches-1538606 ] Patch to fix __index__() clipping
SourceForge.net
noreply at sourceforge.net
Sat Aug 12 12:40:23 CEST 2006
Patches item #1538606, was opened at 2006-08-11 10:51
Message generated for change (Comment added) made by arigo
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1538606&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 9
Submitted By: Travis Oliphant (teoliphant)
Assigned to: Neal Norwitz (nnorwitz)
Summary: Patch to fix __index__() clipping
Initial Comment:
This is a patch that builds off of Nick Coghlan's work
to fix the __index__() clipping problem.
It defines 3 New C-API calls (1 is a macro):
int PyIndex_Check(obj) -- does this object have nb_index
PyObject* PyNumber_Index(obj) -- return nb_index(obj)
if possible
Py_ssize_t PyNumber_AsSsize_t(obj, err) -- return obj
as Py_ssize_t by frist going through nb_index(obj)
which returns an integer or long integer. If err is
NULL, then clip on Overflow, otherwise raise err on
Overflow encountered when trying to fit the result of
nb_index into a Py_ssize_t slot.
With these three C-API calls, I was able to fix all the
problems that have been identified and simplify several
pieces of code.
----------------------------------------------------------------------
>Comment By: Armin Rigo (arigo)
Date: 2006-08-12 10:40
Message:
Logged In: YES
user_id=4771
The check for a negative long needs to be done
differently; this is really depending too much
on internal details.
Note also that the _long_as_ssize_t() function
was introduced to support both long_index() and
_PyLong_AsSsize_t(). Now that the former is
removed, the situation looks slightly strange,
because _long_as_ssize_t() is doing a bit too
much work for its one remaining caller.
That's why somehow reusing _long_as_ssize_t()
from abstract.c would look cleaner to me.
----------------------------------------------------------------------
Comment By: Travis Oliphant (teoliphant)
Date: 2006-08-11 19:16
Message:
Logged In: YES
user_id=5296
I've made the changes Armin suggested. I changed
operator.index to go back to its original behavior of
returning a.__index__()
I'm +0 on adding _PyLong_AsClippedSsize_t to clean-up the
check for a negative long integer.
----------------------------------------------------------------------
Comment By: Armin Rigo (arigo)
Date: 2006-08-11 17:15
Message:
Logged In: YES
user_id=4771
I like this API too, and the patch looks good
apart from some more details:
A style note first: some lines are indented with
spaces instead of tabs. This causes some
changes on otherwise-unchanged lines, too.
if PyIndex_Check(key) => if (PyIndex_Check(key))
typeobject.c: slot_nb_index() may not need to do
the type-checking because it is done anyway in
the caller, which can only be PyNumber_Index().
classobject.c: same remark about instance_index().
unicodeobject.c: kill macro HAS_INDEX
mmapmodule.c: idem
arraymodule.c: idem
should operator.index(o) return
PyNumber_AsSsize_t(o) or just PyNumber_Index(o)?
I can think of use cases for the latter, which
is somehow the most primitive of the two, so it
should IMHO be exposed via the operator module.
docs: "possitive" => "positive"
----------------------------------------------------------------------
Comment By: Nick Coghlan (ncoghlan)
Date: 2006-08-11 11:47
Message:
Logged In: YES
user_id=1038590
The PyNumber_Index docs should explicitly state that it
raises IndexError when overflow occurs.
It may also be worth resurrecting _PyLong_AsClippedSsize_t
in order to clean up the implementation of
PyNumber_AsSsize_t (detecting OverflowError then poking
around in the guts of a long object is a bit ugly).
Other than that, looks good to me (I like this API a lot
better than mine).
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1538606&group_id=5470
More information about the Patches
mailing list