Repository navigation
Conversation
PyObjectSetItem and PyObjectDelItem raised TypeError as soon as the type had neither mp_ass_subscript nor sq_ass_item, without looking at the key. CPython first converts an index key with PyNumber_AsSsize_t(key, IndexError) when the type has a sequence protocol, so an exception from __index__, or IndexError for a key that does not fit in 64 bits, is raised instead. Add that branch to both nodes, checking the 64-bit range rather than GraalPy's 32-bit index size, since the converted value is not used. Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>
This branch has not been deployed
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 #1255
PyObjectSetItemandPyObjectDelItemconverted an index key only for types withsq_ass_item, so on a type with sequence slots but nosq_ass_item(tuple,str,bytes,range,frozenset, a plain class) they raisedTypeErrorwithout calling__index__. CPython'sPyObject_SetItemandPyObject_DelItemconvert the key whenever the type hastp_as_sequence, so an error from__index__, orIndexErrorfor an oversized int, comes first (abstract.c#L237-L252,#L271-L286).AS-IS
TO-BE
(matching CPython)
Changes
doSequenceNoAssItemspecialization toPyObjectSetItemGenericandPyObjectDelItemGenericformp_ass_subscript == null,sq_ass_item == nullandhas_as_sequence(). It converts an index key withPyNumberIndexNode, raisesIndexErrorwhen the result does not fit in 64 bits, and otherwise raises the existingTypeError. The converted value is not used, so the check follows CPython's 64-bitPy_ssize_trather than GraalPy's 32-bit index size, and a key such as2**40keeps raisingTypeError.test_setitem_delitem_convert_index_before_type_errortotests/test_operator.py.Testing
mx python-jvmon darwin-aarch64.tests/test_operator.pypasses (4 tests), and the new test fails on the unpatched build. The taggedlib-pythonsuitestest_operator,test_index,test_list,test_tuple,test_bytes,test_genericalias,test_types,test_userlist,test_dict,test_arrayandtest_dequepass. Setting and deleting with four keys (a raising__index__,2**100,0and"k") on 34 builtin and user-defined objects now gives the same exception class as CPython 3.13.14, as do the boundary keys2**31,2**40,2**63 - 1,2**63,-2**63and-2**63 - 1, apart fromitertools.count, which has no sequence slots in GraalPy. Types without sequence slots, such astypes.GenericAliasandtypes.UnionType, still raiseTypeError.