Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A potential bug in custom string interpolation constructor resolving for ref parameters #73690

Closed
PaulusParssinen opened this issue May 24, 2024 · 1 comment
Assignees
Milestone

Comments

@PaulusParssinen
Copy link

PaulusParssinen commented May 24, 2024

Version Used: VS Version 17.10.0 Preview 7.0 - reproduces against main branch (see SharpLab)

image

As discussed on the C# discord, here's the minimal repro (a left out implementation details, the issue is the binding of the interpolation ctor)

A minimal repro: SharpLab

Diagnostic Ids: CS1620, CS8350, CS8352

Expected Behavior: The instance method binds the interpolated handler constructor similarly to the extension method.

Actual Behavior: Various ref safety diagnostics emitted for the instance variant.

Not critical but it'd be also nice to have same lowering for the instance versions as the extension varitant (while testing various different combinations of ref keywords, I saw a lot of local tmps , while this static extension has none)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels May 24, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 4, 2024
@jaredpar jaredpar added this to the 17.11 milestone Jun 4, 2024
@333fred
Copy link
Member

333fred commented Jun 14, 2024

So, I'm thinking this is not bug (or at least, not an implementation bug. It may be a specification bug.) The constructor resolution section of the interpolated string handler specification says this (emphasis added):

  • Otherwise, the type of every resolved px is added to the argument list, in the order specified by the Arguments array. Each px is passed with the same ref semantics as is specified in M1.

Where px is the parameter of the resolved constructor, and M1 is the method being called (Append_Instance or Append_Extension in this case). The ref semantics of the receiver of an instance method isn't well-specified as far as I can tell; however, I'm somewhat unconvinced it should be allowed to be treated as ref implicitly, given that we may or may not emit dups for some structs. Should this code compile if the builder was a readonly parameter, for example? To make forward progress on this, I think a full rules change for C# would need to be proposed, specifying when a receiver would be allowed to be passed by ref to the constructor of an interpolated string handler; those rules could then be verified and reviewed, and we would have a full spec to test the implementation against. That would need to be opened on csharplang, and I would likely be willing to champion such a feature, but the proposal would need to come from someone else.

@333fred 333fred closed this as not planned Won't fix, can't repro, duplicate, stale Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants