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

Avoid assert in emit layer when intercepted call implicitly captures receiver to temp #74509

Merged
merged 2 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/features/interceptors.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,37 @@ This allows generator authors to avoid *polluting lookup* with interceptors, hel

We may also want to consider adjusting behavior of `[EditorBrowsable]` to work in the same compilation.

### Struct receiver capture

An interceptor whose `this` parameter takes a struct by-reference can generally be used to intercept a struct instance method call, assuming the methods are compatible per [Signature matching](#signature-matching). This includes situations where the receiver must be implicitly captured to temp before the invocation, even if such capture is not permitted when the interceptor is called directly. See also [12.8.9.3 Extension method invocations](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/expressions.md#12893-extension-method-invocations) in the standard.


```cs
using System.Runtime.CompilerServices;

struct S
{
public void Original() { }
}

static class Program
{
public static void Main()
{
new S().Original(); // L1: interception is valid, no errors.
new S().Interceptor(); // error CS1510: A ref or out value must be an assignable variable
}
}

static class D
{
[InterceptsLocation(1, "..(refers to call to 'Original()' at L1)")]
public static void Interceptor(this ref S s)
}
```

The reason we permit implicit receiver capture for the above intercepted call is: we want intercepting to be possible even when the interceptor author doesn't own the original receiver type. If we didn't do this, then intercepting `Original()` in the above example would only be possible by adding instance members to `struct S`.

### Editor experience

Interceptors are treated like a post-compilation step in this design. Diagnostics are given for misuse of interceptors, but some diagnostics are only given in the command-line build and not in the IDE. There is limited traceability in the editor for which calls in a compilation are actually being intercepted. If this feature is brought forward past the experimental stage, this limitation will need to be re-examined.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ private void InterceptCallAndAdjustArguments(
ref BoundExpression? receiverOpt,
ref ImmutableArray<BoundExpression> arguments,
ref ImmutableArray<RefKind> argumentRefKindsOpt,
ref ArrayBuilder<LocalSymbol> temps,
bool invokedAsExtensionMethod,
Syntax.SimpleNameSyntax? nameSyntax)
{
Expand Down Expand Up @@ -281,11 +282,26 @@ private void InterceptCallAndAdjustArguments(
|| (!receiverOpt.Type.IsReferenceType && interceptor.Parameters[0].Type.IsReferenceType));
receiverOpt = MakeConversionNode(receiverOpt, interceptor.Parameters[0].Type, @checked: false, markAsChecked: true);

var thisRefKind = methodThisParameter.RefKind;
// Instance call receivers can be implicitly captured to temps in the emit layer, but not static call arguments
// Therefore we may need to explicitly store the receiver to temp here.
if (thisRefKind != RefKind.None
&& !Binder.HasHome(
receiverOpt,
thisRefKind == RefKind.Ref ? Binder.AddressKind.Writeable : Binder.AddressKind.ReadOnlyStrict,
_factory.CurrentFunction,
peVerifyCompatEnabled: false,
stackLocalsOpt: null))
{
var receiverTemp = _factory.StoreToTemp(receiverOpt, out var assignmentToTemp);
temps.Add(receiverTemp.LocalSymbol);
receiverOpt = _factory.Sequence(locals: [], sideEffects: [assignmentToTemp], receiverTemp);
}

arguments = arguments.Insert(0, receiverOpt);
receiverOpt = null;

// CodeGenerator.EmitArguments requires that we have a fully-filled-out argumentRefKindsOpt for any ref/in/out arguments.
var thisRefKind = methodThisParameter.RefKind;
if (argumentRefKindsOpt.IsDefault && thisRefKind != RefKind.None)
{
argumentRefKindsOpt = method.Parameters.SelectAsArray(static param => param.RefKind);
Expand Down Expand Up @@ -401,7 +417,7 @@ BoundExpression visitArgumentsAndFinishRewrite(BoundCall node, BoundExpression?
ref temps,
invokedAsExtensionMethod);

InterceptCallAndAdjustArguments(ref method, ref rewrittenReceiver, ref rewrittenArguments, ref argRefKindsOpt, invokedAsExtensionMethod, node.InterceptableNameSyntax);
InterceptCallAndAdjustArguments(ref method, ref rewrittenReceiver, ref rewrittenArguments, ref argRefKindsOpt, ref temps, invokedAsExtensionMethod, node.InterceptableNameSyntax);

if (Instrument)
{
Expand Down
Loading