Fix SRTP get_Item witness resolution for string indexers#19757
Conversation
Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/c94502d3-ac80-4c41-af0e-ebfe0317394b Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
get_Item witness resolution for string indexers
T-Gro
left a comment
There was a problem hiding this comment.
LGTM — clean, correct fix.
Root cause analysis: The pattern [stringTy; _; _] expected 3 elements in CompiledObjectAndArgumentTypes, but for string.get_Item(int) the actual shape is [string; int] (2 elements — object type + parameter type). This is consistent with the get_Item array case on the line above which uses [arrTy; _]. The mismatch meant the branch never fired, falling through to MakeBuiltInWitnessInfo which has no get_ItemDynamic case, hence the internal error.
Fix correctness: Changing to [stringTy; _] now matches correctly. Passing [] as tyargs is right because getstring_info (IntrinsicFunctions.GetString) is non-generic. The argExprs forwarding (string instance + int index) matches the [[v_string_ty]; [v_int_ty]] signature.
Test: Appropriate runtime test since the bug manifests at codegen/execution, not type-checking. --nowarn:77 correctly suppresses the special-status-member-constraint warning.
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
|
SRTP calls constrained to
member get_Itemcould hit an internal compiler error onstring(unknown builtin witness 'get_ItemDynamic') instead of resolving to the string indexer. This change aligns string handling with existing builtin witness lowering for indexers.Compiler fix: correct builtin witness shape for string
get_ItemTcGlobals.TryMakeOperatorAsBuiltInWitnessInfoto match the actual compiled argument shape forstringindexer access.get_Itemonstringnow resolves through the existinggetstring_infopath rather than falling through to dynamic builtin witness lookup.Regression coverage
IWSAMsAndSRTPsTestsverifying SRTPmember get_Item: int -> ^Uworks againststringat compile/run time.