| msg50860 - (view) |
Author: Travis Oliphant (teoliphant) *  |
Date: 2006-08-11 10:51 |
| 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. |
|
|
| msg50861 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2006-08-11 11:47 |
| 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). |
|
|
| msg50862 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2006-08-11 17:15 |
| 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" |
|
|
| msg50863 - (view) |
Author: Travis Oliphant (teoliphant) * |
Date: 2006-08-11 19:16 |
| 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. |
|
|
| msg50864 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2006-08-12 10:40 |
| 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. |
|
|
| msg50865 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2006-08-12 14:09 |
| Logged In: YES user_id=1038590 It's probably worth grabbing just the part of my patch that added the _PyLong_AsClippedSsize_t internal interface to longobject.h - this used an output variable to indicate whether or not clipping occurred, making it easy for abstract.c to decide how to proceed. No doubt some modifications would be needed to fit in with Travis's patch, though, and I won't have time to do that this week. |
|
|
| msg50866 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2006-08-12 16:53 |
| Logged In: YES user_id=33168 I've modified this patch (mostly for style) and will be checking in today. Armin, I changed the calc for the sign bit to use _PyLong_Sign(). I don't think this patch is perfect (I added some XXX comments), but it needs to get in sooner rather than later. Please try to review what I check in. |
|
|
| msg50867 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2006-08-12 17:04 |
| Logged In: YES user_id=33168 Committed revision 51236. Please make comments on the checkin. It would probably be best to keep the discussion on python-dev. Thanks! |
|
|