From dc3c8cbb1a8e74454da1eae90b728d93a9f30c98 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Wed, 24 May 2023 12:13:56 -0700 Subject: [PATCH 1/8] Skip checks for overridden or hidden members in overload resolution with extension methods --- .../Portable/Binder/Binder_Expressions.cs | 4 +- .../OverloadResolution/OverloadResolution.cs | 92 +++++++++++-------- .../Semantics/OverloadResolutionPerfTests.cs | 75 +++++++++++++++ 3 files changed, 131 insertions(+), 40 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index f5d1f696306b5..60f6305e8a4bb 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -7397,7 +7397,8 @@ protected MethodGroupResolution BindExtensionMethod( isMethodGroupConversion: isMethodGroupConversion, allowRefOmittedArguments: allowRefOmittedArguments, returnRefKind: returnRefKind, - returnType: returnType); + returnType: returnType, + isExtensionMethodResolution: true); diagnostics.Add(expression, useSiteInfo); var sealedDiagnostics = diagnostics.ToReadOnlyAndFree(); @@ -9151,6 +9152,7 @@ private MethodGroupResolution ResolveDefaultMethodGroup( returnRefKind, returnType, isFunctionPointerResolution, + isExtensionMethodResolution: false, callingConvention); // Note: the MethodGroupResolution instance is responsible for freeing its copy of analyzed arguments diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 0c694247f1f9e..0e00491c7099e 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -128,12 +128,14 @@ public void MethodInvocationOverloadResolution( RefKind returnRefKind = default, TypeSymbol returnType = null, bool isFunctionPointerResolution = false, + bool isExtensionMethodResolution = false, in CallingConventionInfo callingConventionInfo = default) { MethodOrPropertyOverloadResolution( methods, typeArguments, receiver, arguments, result, isMethodGroupConversion, allowRefOmittedArguments, ref useSiteInfo, inferWithDynamic, - allowUnexpandedForm, returnRefKind, returnType, isFunctionPointerResolution, in callingConventionInfo); + allowUnexpandedForm, returnRefKind, returnType, isFunctionPointerResolution: isFunctionPointerResolution, + isExtensionMethodResolution: isExtensionMethodResolution, in callingConventionInfo); } // Perform overload resolution on the given property group, with the given arguments and @@ -168,16 +170,20 @@ internal void MethodOrPropertyOverloadResolution( RefKind returnRefKind = default, TypeSymbol returnType = null, bool isFunctionPointerResolution = false, + bool isExtensionMethodResolution = false, in CallingConventionInfo callingConventionInfo = default) where TMember : Symbol { var results = result.ResultsBuilder; + // No need to check for overridden or hidden methods if the members are extension methods. + bool checkOverriddenOrHidden = !isExtensionMethodResolution; + // First, attempt overload resolution not getting complete results. PerformMemberOverloadResolution( results, members, typeArguments, receiver, arguments, completeResults: false, isMethodGroupConversion, returnRefKind, returnType, allowRefOmittedArguments, isFunctionPointerResolution, callingConventionInfo, - ref useSiteInfo, inferWithDynamic, allowUnexpandedForm); + ref useSiteInfo, inferWithDynamic: inferWithDynamic, allowUnexpandedForm: allowUnexpandedForm, checkOverriddenOrHidden: checkOverriddenOrHidden); if (!OverloadResolutionResultIsValid(results, arguments.HasDynamicArgument)) { @@ -187,7 +193,7 @@ internal void MethodOrPropertyOverloadResolution( results, members, typeArguments, receiver, arguments, completeResults: true, isMethodGroupConversion, returnRefKind, returnType, allowRefOmittedArguments, isFunctionPointerResolution, callingConventionInfo, - ref useSiteInfo, allowUnexpandedForm: allowUnexpandedForm); + ref useSiteInfo, inferWithDynamic: false, allowUnexpandedForm: allowUnexpandedForm, checkOverriddenOrHidden: checkOverriddenOrHidden); } } @@ -239,8 +245,9 @@ private void PerformMemberOverloadResolution( bool isFunctionPointerResolution, in CallingConventionInfo callingConventionInfo, ref CompoundUseSiteInfo useSiteInfo, - bool inferWithDynamic = false, - bool allowUnexpandedForm = true) + bool inferWithDynamic, + bool allowUnexpandedForm, + bool checkOverriddenOrHidden) where TMember : Symbol { // SPEC: The binding-time processing of a method invocation of the form M(A), where M is a @@ -256,7 +263,7 @@ private void PerformMemberOverloadResolution( // overriding or hiding member is not in a subtype of the type containing the // (potentially) overridden or hidden member. Dictionary> containingTypeMapOpt = null; - if (members.Count > 50) // TODO: fine-tune this value + if (checkOverriddenOrHidden && members.Count > 50) // TODO: fine-tune this value { containingTypeMapOpt = PartitionMembersByContainingType(members); } @@ -276,7 +283,8 @@ private void PerformMemberOverloadResolution( containingTypeMapOpt, inferWithDynamic: inferWithDynamic, useSiteInfo: ref useSiteInfo, - allowUnexpandedForm: allowUnexpandedForm); + allowUnexpandedForm: allowUnexpandedForm, + checkOverriddenOrHidden: checkOverriddenOrHidden); } // CONSIDER: use containingTypeMapOpt for RemoveLessDerivedMembers? @@ -824,9 +832,12 @@ private void AddMemberToCandidateSet( Dictionary> containingTypeMapOpt, bool inferWithDynamic, ref CompoundUseSiteInfo useSiteInfo, - bool allowUnexpandedForm) + bool allowUnexpandedForm, + bool checkOverriddenOrHidden = true) where TMember : Symbol { + Debug.Assert(checkOverriddenOrHidden || containingTypeMapOpt is null); + // SPEC VIOLATION: // // The specification states that the method group that resulted from member lookup has @@ -856,47 +867,50 @@ private void AddMemberToCandidateSet( // clever in filtering out methods from less-derived classes later, but we'll cross that // bridge when we come to it. - if (members.Count < 2) + if (checkOverriddenOrHidden) { - // No hiding or overriding possible. - } - else if (containingTypeMapOpt == null) - { - if (MemberGroupContainsMoreDerivedOverride(members, member, checkOverrideContainingType: true, ref useSiteInfo)) + if (members.Count < 2) { - // Don't even add it to the result set. We'll add only the most-overriding members. - return; + // No hiding or overriding possible. } + else if (containingTypeMapOpt == null) + { + if (MemberGroupContainsMoreDerivedOverride(members, member, checkOverrideContainingType: true, ref useSiteInfo)) + { + // Don't even add it to the result set. We'll add only the most-overriding members. + return; + } - if (MemberGroupHidesByName(members, member, ref useSiteInfo)) + if (MemberGroupHidesByName(members, member, ref useSiteInfo)) + { + return; + } + } + else if (containingTypeMapOpt.Count == 1) { - return; + // No hiding or overriding since all members are in the same type. } - } - else if (containingTypeMapOpt.Count == 1) - { - // No hiding or overriding since all members are in the same type. - } - else - { - // NOTE: only check for overriding/hiding in subtypes of f.ContainingType. - NamedTypeSymbol memberContainingType = member.ContainingType; - foreach (var pair in containingTypeMapOpt) + else { - NamedTypeSymbol otherType = pair.Key; - if (otherType.IsDerivedFrom(memberContainingType, TypeCompareKind.ConsiderEverything, useSiteInfo: ref useSiteInfo)) + // NOTE: only check for overriding/hiding in subtypes of f.ContainingType. + NamedTypeSymbol memberContainingType = member.ContainingType; + foreach (var pair in containingTypeMapOpt) { - ArrayBuilder others = pair.Value; - - if (MemberGroupContainsMoreDerivedOverride(others, member, checkOverrideContainingType: false, ref useSiteInfo)) + NamedTypeSymbol otherType = pair.Key; + if (otherType.IsDerivedFrom(memberContainingType, TypeCompareKind.ConsiderEverything, useSiteInfo: ref useSiteInfo)) { - // Don't even add it to the result set. We'll add only the most-overriding members. - return; - } + ArrayBuilder others = pair.Value; - if (MemberGroupHidesByName(others, member, ref useSiteInfo)) - { - return; + if (MemberGroupContainsMoreDerivedOverride(others, member, checkOverrideContainingType: false, ref useSiteInfo)) + { + // Don't even add it to the result set. We'll add only the most-overriding members. + return; + } + + if (MemberGroupHidesByName(others, member, ref useSiteInfo)) + { + return; + } } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 4ee3a6c1809c4..96011f5c7341b 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -780,5 +780,80 @@ public void DefiniteAssignment_ManySwitchCasesAndLabels() // int tmp2; // unused Diagnostic(ErrorCode.WRN_UnreferencedVar, "tmp2").WithArguments("tmp2").WithLocation(9, 13)); } + + [ConditionalFact(typeof(IsRelease))] + [WorkItem("https://github.com/dotnet/roslyn/issues/67926")] + public void ExtensionOverloadsDistinctClasses_01() + { + const int n = 30000; + + var builder = new StringBuilder(); + builder.AppendLine( + $$""" + class Program + { + static void Main() + { + var x = new C0(); + var y = new C{{n / 2}}(); + x.F(y); + } + } + """); + + for (int i = 0; i < n; i++) + { + builder.AppendLine( + $$""" + class C{{i}} { } + static class E{{i}} + { + public static void F(this object x, C{{i}} y) { } + } + """); + } + + string source = builder.ToString(); + var comp = CreateCompilation(source); + comp.VerifyDiagnostics(); + } + + [ConditionalFact(typeof(IsRelease))] + [WorkItem("https://github.com/dotnet/roslyn/issues/67926")] + public void ExtensionOverloadsDistinctClasses_02() + { + const int n = 30000; + + var builder = new StringBuilder(); + builder.AppendLine( + """ + class Program + { + static void Main() + { + new C0().F( + } + } + """); + + for (int i = 0; i < n; i++) + { + builder.AppendLine( + $$""" + class C{{i}} { } + static class E{{i}} + { + public static void F(this object x, C{{i}} y) { } + } + """); + } + + string source = builder.ToString(); + var comp = CreateCompilation(source); + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var expr = tree.GetCompilationUnitRoot().DescendantNodes().OfType().Single(); + _ = model.GetTypeInfo(expr); + } } } From 2067bd8938079b1e421201c3c23eb99d395500e6 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Thu, 25 May 2023 11:48:42 -0700 Subject: [PATCH 2/8] Update tests --- .../Semantics/OverloadResolutionPerfTests.cs | 62 ++++++++++++++++--- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 96011f5c7341b..9a22baeb904fc 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -785,7 +785,7 @@ public void DefiniteAssignment_ManySwitchCasesAndLabels() [WorkItem("https://github.com/dotnet/roslyn/issues/67926")] public void ExtensionOverloadsDistinctClasses_01() { - const int n = 30000; + const int n = 1000; var builder = new StringBuilder(); builder.AppendLine( @@ -794,9 +794,9 @@ class Program { static void Main() { - var x = new C0(); - var y = new C{{n / 2}}(); - x.F(y); + var o = new object(); + var c = new C1(); + o.F(c, c => o.F(c, null)); } } """); @@ -808,7 +808,7 @@ static void Main() class C{{i}} { } static class E{{i}} { - public static void F(this object x, C{{i}} y) { } + public static void F(this object o, C{{i}} c, System.Action a) { } } """); } @@ -822,16 +822,57 @@ public static void F(this object x, C{{i}} y) { } [WorkItem("https://github.com/dotnet/roslyn/issues/67926")] public void ExtensionOverloadsDistinctClasses_02() { - const int n = 30000; + const int n = 1000; var builder = new StringBuilder(); builder.AppendLine( - """ + $$""" + class Program + { + static void Main() + { + var o = new object(); + o.F(null, c => o.F(c, null)); + } + } + """); + + for (int i = 0; i < n; i++) + { + builder.AppendLine( + $$""" + class C{{i}} { } + static class E{{i}} + { + public static void F(this object o, C{{i}} c, System.Action a) { } + } + """); + } + + string source = builder.ToString(); + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (6,11): error CS0121: The call is ambiguous between the following methods or properties: 'E0.F(object, C0, Action)' and 'E1.F(object, C1, Action)' + // o.F(null, c => o.F(c, null)); + Diagnostic(ErrorCode.ERR_AmbigCall, "F").WithArguments("E0.F(object, C0, System.Action)", "E1.F(object, C1, System.Action)").WithLocation(6, 11)); + } + + [ConditionalFact(typeof(IsRelease))] + [WorkItem("https://github.com/dotnet/roslyn/issues/67926")] + public void ExtensionOverloadsDistinctClasses_03() + { + const int n = 1000; + + var builder = new StringBuilder(); + builder.AppendLine( + $$""" class Program { static void Main() { - new C0().F( + var o = new object(); + var c = new C1(); + o.F(c, c => { o.F( }); } } """); @@ -843,7 +884,7 @@ static void Main() class C{{i}} { } static class E{{i}} { - public static void F(this object x, C{{i}} y) { } + public static void F(this object o, C{{i}} c, System.Action a) { } } """); } @@ -852,7 +893,8 @@ public static void F(this object x, C{{i}} y) { } var comp = CreateCompilation(source); var tree = comp.SyntaxTrees.Single(); var model = comp.GetSemanticModel(tree); - var expr = tree.GetCompilationUnitRoot().DescendantNodes().OfType().Single(); + var expr = tree.GetCompilationUnitRoot().DescendantNodes().OfType().Last(); + Assert.Equal("o.F( ", expr.ToString()); _ = model.GetTypeInfo(expr); } } From 2d9f6225551d7f7dda6713aba53e69c2d870aa65 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Tue, 6 Jun 2023 15:22:59 -0700 Subject: [PATCH 3/8] Skip RemoveLessDerivedMembers for extension methods --- .../Semantics/OverloadResolution/OverloadResolution.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 0e00491c7099e..dcb1bbe7fdba5 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -296,7 +296,10 @@ private void PerformMemberOverloadResolution( RemoveInaccessibleTypeArguments(results, ref useSiteInfo); // SPEC: The set of candidate methods is reduced to contain only methods from the most derived types. - RemoveLessDerivedMembers(results, ref useSiteInfo); + if (checkOverriddenOrHidden) + { + RemoveLessDerivedMembers(results, ref useSiteInfo); + } if (Compilation.LanguageVersion.AllowImprovedOverloadCandidates()) { @@ -1656,7 +1659,7 @@ private void RemoveWorseMembers(ArrayBuilder /// Returns the parameter type (considering params). /// - private TypeSymbol GetParameterType(ParameterSymbol parameter, MemberAnalysisResult result) + private static TypeSymbol GetParameterType(ParameterSymbol parameter, MemberAnalysisResult result) { var type = parameter.Type; if (result.Kind == MemberResolutionKind.ApplicableInExpandedForm && From 1fc1bc785721bd29af6e8fa15922956462d11702 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Tue, 6 Jun 2023 15:55:36 -0700 Subject: [PATCH 4/8] Update comment --- .../Binder/Semantics/OverloadResolution/OverloadResolution.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index dcb1bbe7fdba5..475eeca40aae6 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -176,7 +176,8 @@ internal void MethodOrPropertyOverloadResolution( { var results = result.ResultsBuilder; - // No need to check for overridden or hidden methods if the members are extension methods. + // No need to check for overridden or hidden methods + // if the members were resolved as extension methods. bool checkOverriddenOrHidden = !isExtensionMethodResolution; // First, attempt overload resolution not getting complete results. From dc04f0208bbc6f22d49a00fecbdba4ef1064bcdc Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Wed, 7 Jun 2023 16:20:28 -0700 Subject: [PATCH 5/8] Check for multiple valid results --- .../OverloadResolution/OverloadResolution.cs | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 475eeca40aae6..83153eee80d43 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -1267,35 +1267,38 @@ private static void RemoveLessDerivedMembers(ArrayBuilder r.Result.IsValid) > 1) { - var result = results[f]; - - // As in dev12, we want to drop use site errors from less-derived types. - // NOTE: Because of use site warnings, a result with a diagnostic to report - // might not have kind UseSiteError. This could result in a kind being - // switched to LessDerived (i.e. loss of information), but it is the most - // straightforward way to suppress use site diagnostics from less-derived - // members. - if (!(result.Result.IsValid || result.HasUseSiteDiagnosticToReport)) + for (int f = 0; f < results.Count; ++f) { - continue; - } + var result = results[f]; - // Note that we are doing something which appears a bit dodgy here: we're modifying - // the validity of elements of the set while inside an outer loop which is filtering - // the set based on validity. This means that we could remove an item from the set - // that we ought to be processing later. However, because the "is a base type of" - // relationship is transitive, that's OK. For example, suppose we have members - // Cat.M, Mammal.M and Animal.M in the set. The first time through the outer loop we - // eliminate Mammal.M and Animal.M, and therefore we never process Mammal.M the - // second time through the outer loop. That's OK, because we have already done the - // work necessary to eliminate methods on base types of Mammal when we eliminated - // methods on base types of Cat. + // As in dev12, we want to drop use site errors from less-derived types. + // NOTE: Because of use site warnings, a result with a diagnostic to report + // might not have kind UseSiteError. This could result in a kind being + // switched to LessDerived (i.e. loss of information), but it is the most + // straightforward way to suppress use site diagnostics from less-derived + // members. + if (!(result.Result.IsValid || result.HasUseSiteDiagnosticToReport)) + { + continue; + } - if (IsLessDerivedThanAny(result.LeastOverriddenMember.ContainingType, results, ref useSiteInfo)) - { - results[f] = result.WithResult(MemberAnalysisResult.LessDerived()); + // Note that we are doing something which appears a bit dodgy here: we're modifying + // the validity of elements of the set while inside an outer loop which is filtering + // the set based on validity. This means that we could remove an item from the set + // that we ought to be processing later. However, because the "is a base type of" + // relationship is transitive, that's OK. For example, suppose we have members + // Cat.M, Mammal.M and Animal.M in the set. The first time through the outer loop we + // eliminate Mammal.M and Animal.M, and therefore we never process Mammal.M the + // second time through the outer loop. That's OK, because we have already done the + // work necessary to eliminate methods on base types of Mammal when we eliminated + // methods on base types of Cat. + + if (IsLessDerivedThanAny(result.LeastOverriddenMember.ContainingType, results, ref useSiteInfo)) + { + results[f] = result.WithResult(MemberAnalysisResult.LessDerived()); + } } } } From ada84d11df4e3f780428faf22a6b1269dd6c7f27 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Wed, 7 Jun 2023 14:54:08 -0700 Subject: [PATCH 6/8] Check extension method containing type --- .../OverloadResolution/OverloadResolution.cs | 8 +- .../Semantics/OverloadResolutionPerfTests.cs | 81 ++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 83153eee80d43..b4a5893a407f6 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -176,9 +176,11 @@ internal void MethodOrPropertyOverloadResolution( { var results = result.ResultsBuilder; - // No need to check for overridden or hidden methods - // if the members were resolved as extension methods. - bool checkOverriddenOrHidden = !isExtensionMethodResolution; + // No need to check for overridden or hidden methods if the members were + // resolved as extension methods and the extension methods are defined in + // types derived from System.Object. + bool checkOverriddenOrHidden = !(isExtensionMethodResolution && + members.All(static m => m.ContainingSymbol is NamedTypeSymbol { BaseTypeNoUseSiteDiagnostics.SpecialType: SpecialType.System_Object })); // First, attempt overload resolution not getting complete results. PerformMemberOverloadResolution( diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 9a22baeb904fc..33480d96b3bbd 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -4,9 +4,10 @@ #nullable disable -using Microsoft.CodeAnalysis.CSharp.Symbols; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Roslyn.Test.Utilities; +using System.Collections.Immutable; using System.Linq; using System.Text; using Xunit; @@ -897,5 +898,83 @@ public static void F(this object o, C{{i}} c, System.Action a) { } Assert.Equal("o.F( ", expr.ToString()); _ = model.GetTypeInfo(expr); } + + [Fact] + public void ExtensionOverloadsDistinctClasses_04() + { + // public abstract class A + // { + // public static void F1(this object obj) { } + // public static void F3(this object obj) { } + // } + // public abstract class B : A + // { + // public static void F2(this object obj) { } + // public static void F3(this object obj) { } + // } + string sourceA = """ + .assembly extern mscorlib { .ver 4:0:0:0 .publickeytoken = (B7 7A 5C 56 19 34 E0 89) } + .assembly extern System.Core { } + .assembly '<>' + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + } + .class public abstract A + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + .method public hidebysig specialname rtspecialname instance void .ctor() { ret } + .method public static void F1(object o) + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + ret + } + .method public static void F3(object o) + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + ret + } + } + .class public abstract B extends A + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + .method public hidebysig specialname rtspecialname instance void .ctor() { ret } + .method public static void F2(object o) + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + ret + } + .method public static void F3(object o) + { + .custom instance void [System.Core]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) + ret + } + } + """; + var refA = CompileIL(sourceA, prependDefaultHeader: false); + + // The calls to B.F3(o) and o.F3() should bind to B.F3 and should not be + // considered ambiguous with A.F3 because B is derived from A. + string sourceB = """ + class Program + { + static void M(object o) + { + B.F1(o); + B.F2(o); + B.F3(o); + o.F1(); + o.F2(); + o.F3(); + } + } + """; + var comp = CreateCompilation(sourceB, references: new[] { refA }); + comp.VerifyDiagnostics(); + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var exprs = tree.GetRoot().DescendantNodes().OfType().ToImmutableArray(); + var containingTypes = exprs.SelectAsArray(e => model.GetSymbolInfo(e).Symbol.ContainingSymbol).ToTestDisplayStrings(); + Assert.Equal(new[] { "A", "B", "B", "A", "B", "B" }, containingTypes); + } } } From 61623be4274336cd5a42085da485c62a21347556 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Wed, 7 Jun 2023 21:36:08 -0700 Subject: [PATCH 7/8] Check for multiple valid results --- .../OverloadResolution/OverloadResolution.cs | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index b4a5893a407f6..7db9441f15d1a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -1269,48 +1269,50 @@ private static void RemoveLessDerivedMembers(ArrayBuilder r.Result.IsValid) > 1) + for (int f = 0; f < results.Count; ++f) { - for (int f = 0; f < results.Count; ++f) + var result = results[f]; + + // As in dev12, we want to drop use site errors from less-derived types. + // NOTE: Because of use site warnings, a result with a diagnostic to report + // might not have kind UseSiteError. This could result in a kind being + // switched to LessDerived (i.e. loss of information), but it is the most + // straightforward way to suppress use site diagnostics from less-derived + // members. + if (!(result.Result.IsValid || result.HasUseSiteDiagnosticToReport)) { - var result = results[f]; + continue; + } - // As in dev12, we want to drop use site errors from less-derived types. - // NOTE: Because of use site warnings, a result with a diagnostic to report - // might not have kind UseSiteError. This could result in a kind being - // switched to LessDerived (i.e. loss of information), but it is the most - // straightforward way to suppress use site diagnostics from less-derived - // members. - if (!(result.Result.IsValid || result.HasUseSiteDiagnosticToReport)) - { - continue; - } + // Note that we are doing something which appears a bit dodgy here: we're modifying + // the validity of elements of the set while inside an outer loop which is filtering + // the set based on validity. This means that we could remove an item from the set + // that we ought to be processing later. However, because the "is a base type of" + // relationship is transitive, that's OK. For example, suppose we have members + // Cat.M, Mammal.M and Animal.M in the set. The first time through the outer loop we + // eliminate Mammal.M and Animal.M, and therefore we never process Mammal.M the + // second time through the outer loop. That's OK, because we have already done the + // work necessary to eliminate methods on base types of Mammal when we eliminated + // methods on base types of Cat. - // Note that we are doing something which appears a bit dodgy here: we're modifying - // the validity of elements of the set while inside an outer loop which is filtering - // the set based on validity. This means that we could remove an item from the set - // that we ought to be processing later. However, because the "is a base type of" - // relationship is transitive, that's OK. For example, suppose we have members - // Cat.M, Mammal.M and Animal.M in the set. The first time through the outer loop we - // eliminate Mammal.M and Animal.M, and therefore we never process Mammal.M the - // second time through the outer loop. That's OK, because we have already done the - // work necessary to eliminate methods on base types of Mammal when we eliminated - // methods on base types of Cat. - - if (IsLessDerivedThanAny(result.LeastOverriddenMember.ContainingType, results, ref useSiteInfo)) - { - results[f] = result.WithResult(MemberAnalysisResult.LessDerived()); - } + if (IsLessDerivedThanAny(index: f, result.LeastOverriddenMember.ContainingType, results, ref useSiteInfo)) + { + results[f] = result.WithResult(MemberAnalysisResult.LessDerived()); } } } // Is this type a base type of any valid method on the list? - private static bool IsLessDerivedThanAny(TypeSymbol type, ArrayBuilder> results, ref CompoundUseSiteInfo useSiteInfo) + private static bool IsLessDerivedThanAny(int index, TypeSymbol type, ArrayBuilder> results, ref CompoundUseSiteInfo useSiteInfo) where TMember : Symbol { for (int f = 0; f < results.Count; ++f) { + if (f == index) + { + continue; + } + var result = results[f]; if (!result.Result.IsValid) From f684fb696d293d548ba5683b66f7decebc3ea143 Mon Sep 17 00:00:00 2001 From: Charles Stoner <10732005+cston@users.noreply.github.com> Date: Fri, 9 Jun 2023 16:52:52 -0700 Subject: [PATCH 8/8] Check diagnostics --- .../Semantic/Semantics/OverloadResolutionPerfTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs index 33480d96b3bbd..b70642d750407 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionPerfTests.cs @@ -892,6 +892,17 @@ public static void F(this object o, C{{i}} c, System.Action a) { } string source = builder.ToString(); var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (7,25): error CS1501: No overload for method 'F' takes 0 arguments + // o.F(c, c => { o.F( }); + Diagnostic(ErrorCode.ERR_BadArgCount, "F").WithArguments("F", "0").WithLocation(7, 25), + // (7,28): error CS1026: ) expected + // o.F(c, c => { o.F( }); + Diagnostic(ErrorCode.ERR_CloseParenExpected, "}").WithLocation(7, 28), + // (7,28): error CS1002: ; expected + // o.F(c, c => { o.F( }); + Diagnostic(ErrorCode.ERR_SemicolonExpected, "}").WithLocation(7, 28)); + var tree = comp.SyntaxTrees.Single(); var model = comp.GetSemanticModel(tree); var expr = tree.GetCompilationUnitRoot().DescendantNodes().OfType().Last();