-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Fix intercepting a struct or enum base method #71655
Conversation
// Program.cs(16,6): error CS9148: Interceptor must have a 'this' parameter matching parameter 'ref Program this' on 'Program.InterceptableMethod()'. | ||
// [InterceptsLocation("Program.cs", 10, 23)] | ||
Diagnostic(ErrorCode.ERR_InterceptorMustHaveMatchingThisParameter, @"InterceptsLocation(""Program.cs"", 10, 23)").WithArguments("ref Program this", "Program.InterceptableMethod()").WithLocation(16, 6)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
class C | ||
{ | ||
public static void M<T>(T t) where T : struct, I |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or alternatively, perhaps remove all constraints and test a call to a virtual method on object
.
static void M<T>(T t)
{
t.ToString();
}
@jcouv for second review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks (iteration 2)
@@ -272,7 +272,11 @@ private void EmbedIfNeedTo(BoundExpression receiver, ImmutableArray<PropertySymb | |||
if (needToReduce) | |||
{ | |||
Debug.Assert(methodThisParameter is not null); | |||
arguments = arguments.Insert(0, receiverOpt!); | |||
Debug.Assert(receiverOpt?.Type is not null); | |||
Debug.Assert(receiverOpt.Type.Equals(interceptor.Parameters[0].Type, TypeCompareKind.AllIgnoreOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try to add a clarifying comment here in a follow-up PR.
Closes #70841
When the original method is an instance method and interceptor is an extension, we have to adjust the receiver argument to pass it for the extension 'this' parameter. Receiver emit strategies such as 'ConstrainedCallVirt' are not available for an extension 'this' parameter, so we may need to insert an additional (generally boxing) conversion, to make the receiver argument actually match the parameter type.
See https://github.com/dotnet/roslyn/blob/main/docs/features/interceptors.md#signature-matching
In test
InterceptorEnumBaseMethod
, for example, we lower the callvalue.ToString()
as if it were callingOtherToString
in reduced form. In that case,value
must be boxed.