-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Only allow static methods for applicable members #3281
Conversation
This fixes a very similar problem to dotnet#3277, where this code is unable to be resolved: ```cs interface I1{} interface I2{} public unsafe class C : I1, I2 { void M(I1 i) {} static void M(I2 i) {} public void M1() { delegate*<C, void> a = M; // Ambiguous because both M's are applicable } } ``` With this change, the instance method M is not applicable, so there is no ambiguity.
@@ -176,6 +176,7 @@ In an unsafe context, an implicit conversion exists from an address-of expressio | |||
- A single method `M` is selected corresponding to a method invocation of the form `E(A)` with the following modifications: | |||
- The arguments list `A` is a list of expressions, each classified as a variable and with the type and modifier (`ref`, `out`, or `in`) of the corresponding _formal\_parameter\_list_ of `D`. | |||
- The candidate methods are only those methods that are only those methods that are applicable in their normal form, not those applicable in their expanded form. | |||
- The candidate methods are only those methods that are static. |
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.
It looks like the previous bullet should be adjusted: "those methods that are only those methods that are applicable"
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 (iteration 1)
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.
Doesn't this preclude allowing binding to instance methods in the future or would that likely involve some special syntax (hypothetically |
It does preclude that, yes. If we really need to we can introduce a |
Easy yes, but possibly not performant, as it will force an additional indirection. (I don't think its needed for v1, just wanted to ensure I understood the state we'd be in and what might be needed in the future to enable it). |
This fixes a very similar problem to #3277, where this code is unable to be resolved:
With this change, the instance method M is not applicable, so there is no ambiguity.