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 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
