Skip to content

Commit

Permalink
Skip checks for overridden or hidden members in overload resolution w…
Browse files Browse the repository at this point in the history 
…ith extension methods (dotnet#68316)
  • Loading branch information
@cston @mavasani
cston authored and mavasani committed Jun 10, 2023
1 parent 20f6ef9 commit 43453dc
Show file tree
Hide file tree
Showing 4 changed files with 286 additions and 58 deletions.
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
111 changes: 68 additions & 43 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -168,16 +170,23 @@ internal void MethodOrPropertyOverloadResolution<TMember>(
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 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(
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))
{
Expand All @@ -187,7 +196,7 @@ internal void MethodOrPropertyOverloadResolution<TMember>(
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);
}
}

Expand Down Expand Up @@ -239,8 +248,9 @@ private void PerformMemberOverloadResolution<TMember>(
bool isFunctionPointerResolution,
in CallingConventionInfo callingConventionInfo,
ref CompoundUseSiteInfo<AssemblySymbol> 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
Expand All @@ -256,7 +266,7 @@ private void PerformMemberOverloadResolution<TMember>(
// overriding or hiding member is not in a subtype of the type containing the
// (potentially) overridden or hidden member.
Dictionary<NamedTypeSymbol, ArrayBuilder<TMember>> containingTypeMapOpt = null;
if (members.Count > 50) // TODO: fine-tune this value
if (checkOverriddenOrHidden && members.Count > 50) // TODO: fine-tune this value
{
containingTypeMapOpt = PartitionMembersByContainingType(members);
}
Expand All @@ -276,7 +286,8 @@ private void PerformMemberOverloadResolution<TMember>(
containingTypeMapOpt,
inferWithDynamic: inferWithDynamic,
useSiteInfo: ref useSiteInfo,
allowUnexpandedForm: allowUnexpandedForm);
allowUnexpandedForm: allowUnexpandedForm,
checkOverriddenOrHidden: checkOverriddenOrHidden);
}

// CONSIDER: use containingTypeMapOpt for RemoveLessDerivedMembers?
Expand All @@ -288,7 +299,10 @@ private void PerformMemberOverloadResolution<TMember>(
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())
{
Expand Down Expand Up @@ -824,9 +838,12 @@ private void AddMemberToCandidateSet<TMember>(
Dictionary<NamedTypeSymbol, ArrayBuilder<TMember>> containingTypeMapOpt,
bool inferWithDynamic,
ref CompoundUseSiteInfo<AssemblySymbol> 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
Expand Down Expand Up @@ -856,47 +873,50 @@ private void AddMemberToCandidateSet<TMember>(
// 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)
{
// No hiding or overriding possible.
}
else if (containingTypeMapOpt == null)
if (checkOverriddenOrHidden)
{
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<TMember> 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<TMember> 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;
}
}
}
}
Expand Down Expand Up @@ -1275,19 +1295,24 @@ private static void RemoveLessDerivedMembers<TMember>(ArrayBuilder<MemberResolut
// 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))
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<TMember>(TypeSymbol type, ArrayBuilder<MemberResolutionResult<TMember>> results, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private static bool IsLessDerivedThanAny<TMember>(int index, TypeSymbol type, ArrayBuilder<MemberResolutionResult<TMember>> results, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
where TMember : Symbol
{
for (int f = 0; f < results.Count; ++f)
{
if (f == index)
{
continue;
}

var result = results[f];

if (!result.Result.IsValid)
Expand Down Expand Up @@ -1642,7 +1667,7 @@ private void RemoveWorseMembers<TMember>(ArrayBuilder<MemberResolutionResult<TMe
/// <summary>
/// Returns the parameter type (considering params).
/// </summary>
private TypeSymbol GetParameterType(ParameterSymbol parameter, MemberAnalysisResult result)
private static TypeSymbol GetParameterType(ParameterSymbol parameter, MemberAnalysisResult result)
{
var type = parameter.Type;
if (result.Kind == MemberResolutionKind.ApplicableInExpandedForm &&
Expand Down
Loading

0 comments on commit 43453dc

Please sign in to comment.