Skip to content

Adjust overload resolution priority from LDM #74428

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.

Sign up for GitHub

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

Merged
333fred merged 1 commit into from
Jul 18, 2024
Merged

Adjust overload resolution priority from LDM #74428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,10 +1712,15 @@ private int GetTheBestCandidateIndex<TMember>(ArrayBuilder<MemberResolutionResul
return currentBestIndex;
}

private static void RemoveLowerPriorityMembers<TMemberResolution, TMember>(ArrayBuilder<TMemberResolution> results)
private void RemoveLowerPriorityMembers<TMemberResolution, TMember>(ArrayBuilder<TMemberResolution> results)
Copy link
Member

@jcouv jcouv Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also incorporate the change into the spec

where TMemberResolution : IMemberResolutionResultWithPriority<TMember>
where TMember : Symbol
{
if (!Compilation.IsFeatureEnabled(MessageID.IDS_OverloadResolutionPriority))
{
return;
}

// - Then, the reduced set of candidate members is grouped by declaring type. Within each group:
// - Candidate function members are ordered by *overload_resolution_priority*.
// - All members that have a lower *overload_resolution_priority* than the highest found within its declaring type group are removed.
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,6 @@ MethodKind is MethodKind.Ordinary
or MethodKind.Constructor
or MethodKind.UserDefinedOperator
or MethodKind.ReducedExtension
or MethodKind.LocalFunction
&& !IsOverride;

#region IMethodSymbolInternal
Expand Down
29 changes: 20 additions & 9 deletions src/Compilers/CSharp/Test/Emit3/OverloadResolutionPriorityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ public class C
{
[OverloadResolutionPriority(1)]
public void M(object o) => System.Console.Write("1");
public void M(string s) => throw null;
public void M(string s) => System.Console.Write("5");

[OverloadResolutionPriority(1)]
public int this[object o]
Expand All @@ -835,8 +835,15 @@ public int this[object o]
}
public int this[string s]
{
get => throw null;
set => throw null;
get
{
System.Console.Write("6");
return 1;
}
set
{
System.Console.Write("7");
}
}

[OverloadResolutionPriority(1)]
Expand All @@ -847,7 +854,7 @@ public C(object o)

public C(string s)
{
throw null;
System.Console.Write("8");
}
}
""";
Expand All @@ -859,9 +866,9 @@ public C(string s)
// (9,6): error CS9202: Feature 'overload resolution priority' is not available in C# 12.0. Please use language version 13.0 or greater.
// [OverloadResolutionPriority(1)]
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion12, "OverloadResolutionPriority(1)").WithArguments("overload resolution priority", "13.0").WithLocation(9, 6),
// (28,6): error CS9202: Feature 'overload resolution priority' is not available in C# 12.0. Please use language version 13.0 or greater.
// (35,6): error CS9202: Feature 'overload resolution priority' is not available in C# 12.0. Please use language version 13.0 or greater.
// [OverloadResolutionPriority(1)]
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion12, "OverloadResolutionPriority(1)").WithArguments("overload resolution priority", "13.0").WithLocation(28, 6));
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion12, "OverloadResolutionPriority(1)").WithArguments("overload resolution priority", "13.0").WithLocation(35, 6));

var definingComp = CreateCompilation([source, OverloadResolutionPriorityAttributeDefinition], parseOptions: TestOptions.Regular13).VerifyDiagnostics();

Expand All @@ -872,8 +879,8 @@ public C(string s)
c.M("test");
""";

// We don't error on consumption, only on definition, so this runs just fine.
CompileAndVerify(consumingSource, references: [definingComp.ToMetadataReference()], parseOptions: TestOptions.Regular12, expectedOutput: "4321").VerifyDiagnostics();
CompileAndVerify(consumingSource, references: [definingComp.ToMetadataReference()], parseOptions: TestOptions.Regular12, expectedOutput: "8765").VerifyDiagnostics();
CompileAndVerify(consumingSource, references: [definingComp.ToMetadataReference()], parseOptions: TestOptions.Regular13, expectedOutput: "4321").VerifyDiagnostics();
}

[Fact]
Expand Down Expand Up @@ -1822,7 +1829,11 @@ public static void Main()
}
""";

CompileAndVerify([source, OverloadResolutionPriorityAttributeDefinition], expectedOutput: "1").VerifyDiagnostics();
CreateCompilation([source, OverloadResolutionPriorityAttributeDefinition]).VerifyDiagnostics(
// (9,10): error CS9262: Cannot use 'OverloadResolutionPriorityAttribute' on this member.
// [OverloadResolutionPriority(1)]
Diagnostic(ErrorCode.ERR_CannotApplyOverloadResolutionPriorityToMember, "OverloadResolutionPriority(1)").WithLocation(9, 10)
);
}

[Fact]
Expand Down