-
Notifications
You must be signed in to change notification settings - Fork 1.8k
C#: Fix extraction of qualified delegate calls #15484
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
C#: Fix extraction of qualified delegate calls #15484
Conversation
d74cac2
to
c74bc68
Compare
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.
Looks good to me!
How/why did you discover this problem?
@@ -33,7 +37,7 @@ protected override void PopulateExpression(TextWriter trapFile) | |||
var target = TargetSymbol; | |||
switch (Syntax.Expression) | |||
{ | |||
case MemberAccessExpressionSyntax memberAccess: | |||
case MemberAccessExpressionSyntax memberAccess when Kind == ExprKind.METHOD_INVOCATION || IsEventDelegateCall() || IsExplicitDelegateInvokeCall(): |
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.
When is this when
condition not the same as Kind == ExprKind.METHOD_INVOCATION || Kind = ExprKind.DELEGATE_INVOCATION
?
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.
Exactly when the member being accessed is not an event or a direct .Invoke
, i.e., when it is a field or a property.
|
||
void M() | ||
{ | ||
this.Field(0); |
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.
Do we have the same problem with function pointers? (unsafe delegate*<int, void> Field2;
)
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.
We did, yes, but that is also fixed by this PR. I have added additional tests to witness.
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
This PR fixes an extractor bug where we would incorrectly determine the qualifier of a delegate call when it was a qualified expression. For example, in
where
LambdaField
is a field holding a lambda, we would considerthis
to be the qualifier of the delegate call, instead ofthis.LambdaField
.