-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Relax signature collision rules for static extension methods #77603
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
Conversation
Specifically, two static extension methods are allowed to have the same signature (according to the current language rules) as long as all the following conditions are met: - methods have different return type and/or different return ref-ness - methods extending different types (ref-ness of the receiver parameter is not considered)
|
|
||
| staticExtensionImplementationsWithoutReturn ??= new Dictionary<MethodSymbol, OneOrMany<MethodSymbol>>(MemberSignatureComparer.DuplicateSourceComparer); | ||
|
|
||
| if (staticExtensionImplementationsWithoutReturn.TryGetValue(staticExtension, out OneOrMany<MethodSymbol> previousStaticExtension)) |
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.
jcouv
left a comment
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 1)
|
@jjonescz, @dotnet/roslyn-compiler For the second review |
| // static public string M2(int x) => throw null; | ||
| Diagnostic(ErrorCode.ERR_MemberAlreadyExists, "M2").WithArguments("M2", "Extensions").WithLocation(12, 30) | ||
| ); | ||
| } |
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 expect this error?
[Fact]
public void TODO2()
{
var src = """
static class E
{
extension(object)
{
public void Method() { }
public static void Method() { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (6,28): error CS0111: Type 'E.extension(object)' already defines a member called 'Method' with the same parameter types
// public static void Method() { }
Diagnostic(ErrorCode.ERR_MemberAlreadyExists, "Method").WithArguments("Method", "E.extension(object)").WithLocation(6, 28));
}
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 expect this error?
Given the current rules, we do. This looks like M9 scenario in SignatureConflict_02. There is a PROTOTYPE comment regarding the current behavior
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.
The M9 scenario in SignatureConflict_02 is:
extension(int receiver)
{
public void M9() {}
}
extension(int receiver)
{
public static void M9(int x) {}
}
M9 generates conflicting signatures in metadata, so should generate an error.
But in the scenario above, the implementation methods have different signatures, so I would expect no error.
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.
so I would expect no error.
Are both methods static extensions?
Specifically, two static extension methods are allowed to have the same signature (according to the current language rules) as long as all the following conditions are met:
Relates to test plan #76130