Skip to content

Fix SRTP get_Item witness resolution for string indexers#19757

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-srtp-member-get-item-error
Draft

Fix SRTP get_Item witness resolution for string indexers#19757
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-srtp-member-get-item-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

SRTP calls constrained to member get_Item could hit an internal compiler error on string (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_Item

    • Updated TcGlobals.TryMakeOperatorAsBuiltInWitnessInfo to match the actual compiled argument shape for string indexer access.
    • get_Item on string now resolves through the existing getstring_info path rather than falling through to dynamic builtin witness lookup.
  • Regression coverage

    • Added a focused component test in IWSAMsAndSRTPsTests verifying SRTP member get_Item: int -> ^U works against string at compile/run time.
let inline indexInto (slice: ^T when ^T: (member get_Item: int -> ^U)) i : ^U =
    slice.get_Item i

let c = indexInto "abcde" 2   // now resolves correctly to string indexer

Copilot AI changed the title [WIP] Fix SRTP internal error when using member get_Item on strings Fix SRTP get_Item witness resolution for string indexers May 18, 2026
Copilot finished work on behalf of T-Gro May 18, 2026 11:52
Copilot AI requested a review from T-Gro May 18, 2026 11:52
Copy link
Copy Markdown
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@T-Gro T-Gro self-requested a review May 20, 2026 12:45
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

❗ Release notes required

@copilot,

Caution

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:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

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

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md No release notes found or release notes format is not correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewed PR reviewed by AI review council

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

SRTP "member get_Item" causes "FS0192: internal error" when used on strings

2 participants