Repository navigation
[mypyc] Make tuple struct names unambiguous - #22472
Open
Jahongir-Qurbonov wants to merge 1 commit into
Open
Jahongir-Qurbonov wants to merge 1 commit into
Jahongir-Qurbonov wants to merge 1 commit into
Conversation
RTuple C struct names are built as "T<length><item codes>", but signed native ints used the digit codes "8", "4", "2" and "1". A digit item code right after the length makes the name ambiguous, so two different tuple types could map to the same struct name. For example, both Tuple[Tuple[i8], Tuple[int, ...11 ints]] and Tuple[Tuple[Tuple[i8], ...10 ints], int] were named "tuple_T2T11T11IIIIIIIIIII", which caused a C compile error. The same could happen with i16, i32 and i64 items. Use "S8", "S4", "S2" and "S1" for signed native ints, mirroring the existing "U8" etc. codes for unsigned ints. Now no item code starts with a digit, so the length is always delimited. Names of tuples without signed native ints are unchanged. Update the vec pop result struct names in librt_vecs.h accordingly.
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.
The C struct name of an RTuple is built as
T<length><item codes>, but signed native ints used the digit codes8,4,2and1. When such a code comes right after a tuple length, the name is ambiguous, so two different tuple types can map to the same struct name. Example:Both return types were named
tuple_T2T11T11IIIIIIIIIII. The C compiler then failed with errors likeincompatible types when assigning to type 'tuple_T11' from type 'tuple_T11T11IIIIIIIIII'. It needs a tuple with at least 10 items, so it's unlikely in practice. The i8 code from #22447 made it a bit easier to hit, but i16, i32 and i64 items could already trigger it with longer tuples.This PR changes the signed native int codes to
S8,S4,S2andS1, to match the existingU8/U4/U2/U1codes for unsigned ints. No item code starts with a digit anymore, so the length always ends at the first non-digit and every name decodes in only one way. Names of tuples without signed native ints stay the same. The only hardcoded names that change are the vec pop result structs inlibrt_vecs.h(tuple_T2V88becomestuple_T2VS8S8, and likewise for 32 and 16 bits). Only the typedef names change, not the struct layouts.I updated the expected names in
test_tuplename.pyand added a unit test and a run test with the colliding pair above.