Fix enumerate() start handling for None and __index__ arguments - #1075
Open
jseop-lim wants to merge 2 commits into
Open
Fix enumerate() start handling for None and __index__ arguments#1075jseop-lim wants to merge 2 commits into
enumerate() start handling for None and __index__ arguments#1075jseop-lim wants to merge 2 commits into
Conversation
The tp_new specializations do not separate an omitted start from an explicit start=None: - doNone dispatched on PNone, which covers both PNone.NO_VALUE (argument omitted) and PNone.NONE (explicit start=None), so enumerate(it, None) silently used 0. CPython defaults start to 0 only when the argument is omitted; anything passed goes through PyNumber_Index and raises. - The rejecting specialization matched PNone.NO_VALUE too. Since the specializations are shared across call sites, guarding only doNone would make a later call that omitted start take that branch and raise instead of using 0, so both guards are needed. Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>
The tp_new specialization set decided whether a start argument was acceptable with a Java type test: isIntegerIndex admits only Integer, Long and PInt, and everything else was rejected without the object being consulted. So bool and any object implementing __index__ raised TypeError. CPython runs a start it was actually given through PyNumber_Index, which takes bool on the PyLong_Check fast path and calls nb_index on anything else providing it. range already does this in GraalPy, via PyNumberIndexNode; enumerate now does too, with the coercion running before the iterator is acquired so argument validation keeps CPython's order. Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1074
enumerate()mishandles itsstartargument in two independent ways. An explicitstart=Noneis treated as if the argument had been omitted, so it silently means 0. And astartthat is not already anintis rejected instead of being coerced, sobooland every object implementing__index__raiseTypeError.AS-IS
TO-BE
(matching CPython)
The two halves ship together because the specializations are shared across call sites. An omitted
startarrives asPNone.NO_VALUE, which is neither an integer index nor distinguishable fromPNone.NONEby Java type, so guarding one specialization without the other moves the failure rather than removing it:enumerate("a", None)enumerate("abc")[(0, 'a'), (1, 'b'), (2, 'c')][(0, 'a'), (1, 'b'), (2, 'c')]Each column is a build. The same branch is already reachable before this change by rejecting a bad
startfirst —enumerate("abc", "x")in atry, thenenumerate("abc").rangealready coerces its arguments throughPyNumberIndexNodeand separates an omitted argument withisNoValueguards, so this bringsenumerateto the same shape.Changes
startto 0 withisNoValue(keywordArg)so it matches only an omitted argument, and rename itdoNoValueto match.doGeneric, which coercesstartthroughPyNumberIndexNodeandCastToJavaLongExactNodebefore the iterator is acquired. Its failure path raises the sameErrorMessages.OBJ_CANNOT_BE_INTERPRETED_AS_INTEGERas before, soNoneandfloatkeep their exact message.!isNoValue(start)to that specialization so an omitted argument can never reach it.tests/test_enumerate_start.pycovering the explicitNone,bool, an__index__implementer, one whose__index__overflows along, a rejectedfloat, and an omittedstartfollowing each of the rejected and coerced cases.Testing
mx buildon linux-aarch64, thenmx graalpytest test_enumerate_start.py test_enumerate.py test_list.py test_tuple.py test_iterator.py— 243 tests, all pass. CPython's owntest.test_enumeratefromlib-pythonpasses as well (92 tests, 14 skipped).Twelve call shapes were compared against CPython 3.12.10 on the built binary, including the sequences above that exercise the shared specialization state and an
__index__that returns2 ** 70; all twelve agree. The table above was measured by building the unpatched tree, the one-guard variant, and the full change, and running the sequence against each.