Skip to content

Commit

Permalink
Only use methods that are fully substituted and pass constraint checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Nov 3, 2023
1 parent 5236b98 commit 4ed1916
Show file tree
Hide file tree
Showing 5 changed files with 641 additions and 130 deletions.
166 changes: 102 additions & 64 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9869,98 +9869,136 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
return GetUniqueSignatureFromMethodGroup_CSharp10(node);
}

MethodSymbol? method = selectMethodForSignature(node);

if (method is null)
MethodSymbol? foundMethod = null;
var typeArguments = node.TypeArgumentsOpt;
if (node.ResultKind == LookupResultKind.Viable)
{
return null;
}
foreach (var instanceMethod in node.Methods)
{
switch (node.ReceiverOpt)
{
case BoundTypeExpression:
case null: // if `using static Class` is in effect, the receiver is missing
if (!instanceMethod.IsStatic) continue;
break;
case BoundThisReference { WasCompilerGenerated: true }:
break;
default:
if (instanceMethod.IsStatic) continue;
break;
}

int arity = node.TypeArgumentsOpt.IsDefaultOrEmpty ? 0 : node.TypeArgumentsOpt.Length;
if (method.Arity != arity)
{
return null;
}
else if (arity > 0)
{
return method.ConstructedFrom.Construct(node.TypeArgumentsOpt);
}
int arity = typeArguments.IsDefaultOrEmpty ? 0 : typeArguments.Length;
if (instanceMethod.Arity != arity)
{
// We have no way of inferring type arguments, so if the given type arguments
// don't match the method's arity, the method is not a candidate
continue;
}

return method;
var substituted = typeArguments.IsDefaultOrEmpty ? instanceMethod : instanceMethod.Construct(typeArguments);
if (!satisfiesConstraintChecks(substituted))
{
continue;
}

static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
{
if (method is null)
{
method = candidate;
return true;
if (!isCandidateUnique(ref foundMethod, substituted))
{
return null;
}
}
if (MemberSignatureComparer.MethodGroupSignatureComparer.Equals(method, candidate))

if (foundMethod is not null)
{
return true;
return foundMethod;
}
method = null;
return false;
}

MethodSymbol? selectMethodForSignature(BoundMethodGroup node)
if (node.SearchExtensionMethods)
{
MethodSymbol? method = null;
if (node.ResultKind == LookupResultKind.Viable)
var receiver = node.ReceiverOpt!;
var methodGroup = MethodGroup.GetInstance();
foreach (var scope in new ExtensionMethodScopes(this))
{
foreach (var m in node.Methods)
methodGroup.Clear();
PopulateExtensionMethodsFromSingleBinder(scope, methodGroup, node.Syntax, receiver, node.Name, typeArguments, BindingDiagnosticBag.Discarded);
foreach (var extensionMethod in methodGroup.Methods)
{
switch (node.ReceiverOpt)
var substituted = typeArguments.IsDefaultOrEmpty ? extensionMethod : extensionMethod.Construct(typeArguments);

var reduced = substituted.ReduceExtensionMethod(receiver.Type, Compilation, out bool wasFullyInferred);
if (reduced is null)
{
case BoundTypeExpression:
case null: // if `using static Class` is in effect, the receiver is missing
if (!m.IsStatic) continue;
break;
case BoundThisReference { WasCompilerGenerated: true }:
break;
default:
if (m.IsStatic) continue;
break;
// Extension method was not applicable
continue;
}

if (!isCandidateUnique(ref method, m))
if (typeArguments.IsDefaultOrEmpty && !wasFullyInferred)
{
// Extension method was not fully inferred
continue;
}

if (!satisfiesConstraintChecks(reduced))
{
continue;
}

var wasUnique = isCandidateUnique(ref foundMethod, reduced);
if (!wasUnique)
{
methodGroup.Free();
return null;
}
}

if (method is not null)
if (foundMethod is not null)
{
return method;
methodGroup.Free();
return foundMethod;
}
}
methodGroup.Free();
}

if (node.SearchExtensionMethods)
return null;

static bool isCandidateUnique(ref MethodSymbol? foundMethod, MethodSymbol candidate)
{
if (foundMethod is null)
{
var receiver = node.ReceiverOpt!;
foreach (var scope in new ExtensionMethodScopes(this))
{
var methodGroup = MethodGroup.GetInstance();
PopulateExtensionMethodsFromSingleBinder(scope, methodGroup, node.Syntax, receiver, node.Name, node.TypeArgumentsOpt, BindingDiagnosticBag.Discarded);
foreach (var m in methodGroup.Methods)
{
if (m.ReduceExtensionMethod(receiver.Type, Compilation) is { } reduced &&
!isCandidateUnique(ref method, reduced))
{
methodGroup.Free();
return null;
}
}
methodGroup.Free();
foundMethod = candidate;
return true;
}
if (MemberSignatureComparer.MethodGroupSignatureComparer.Equals(foundMethod, candidate))
{
return true;
}
foundMethod = null;
return false;
}

if (method is not null)
{
return method;
}
}
bool satisfiesConstraintChecks(MethodSymbol method)
{
if (method.Arity == 0)
{
return true;
}

return null;
var diagnosticsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance();
ArrayBuilder<TypeParameterDiagnosticInfo>? useSiteDiagnosticsBuilder = null;

bool constraintsSatisfied = ConstraintsHelper.CheckMethodConstraints(
method,
new ConstraintsHelper.CheckConstraintsArgs(this.Compilation, this.Conversions, includeNullability: false, location: NoLocation.Singleton, diagnostics: null),
diagnosticsBuilder,
nullabilityDiagnosticsBuilderOpt: null,
ref useSiteDiagnosticsBuilder);

diagnosticsBuilder.Free();
useSiteDiagnosticsBuilder?.Free();

return constraintsSatisfied;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private void InterceptCallAndAdjustArguments(
// we need to take special care to intercept with the extension method as though it is being called in reduced form.
Debug.Assert(receiverOpt is not BoundTypeExpression || method.IsStatic);
var needToReduce = receiverOpt is not (null or BoundTypeExpression) && interceptor.IsExtensionMethod;
var symbolForCompare = needToReduce ? ReducedExtensionMethodSymbol.Create(interceptor, receiverOpt!.Type, _compilation) : interceptor;
var symbolForCompare = needToReduce ? ReducedExtensionMethodSymbol.Create(interceptor, receiverOpt!.Type, _compilation, out _) : interceptor;

if (!MemberSignatureComparer.InterceptorsComparer.Equals(method, symbolForCompare))
{
Expand Down
10 changes: 8 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,14 +733,19 @@ public override TResult Accept<TResult>(CSharpSymbolVisitor<TResult> visitor)
return visitor.VisitMethod(this);
}

public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType, CSharpCompilation compilation)
{
return ReduceExtensionMethod(receiverType, compilation, wasFullyInferred: out _);
}

/// <summary>
/// If this is an extension method that can be applied to a receiver of the given type,
/// returns a reduced extension method symbol thus formed. Otherwise, returns null.
/// </summary>
/// <param name="compilation">The compilation in which constraints should be checked.
/// Should not be null, but if it is null we treat constraints as we would in the latest
/// language version.</param>
public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType, CSharpCompilation compilation)
public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType, CSharpCompilation compilation, out bool wasFullyInferred)
{
if ((object)receiverType == null)
{
Expand All @@ -749,10 +754,11 @@ public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType, CSharpCompila

if (!this.IsExtensionMethod || this.MethodKind == MethodKind.ReducedExtension || receiverType.IsVoidType())
{
wasFullyInferred = false;
return null;
}

return ReducedExtensionMethodSymbol.Create(this, receiverType, compilation);
return ReducedExtensionMethodSymbol.Create(this, receiverType, compilation, out wasFullyInferred);
}

/// <summary>
Expand Down
14 changes: 10 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ internal sealed class ReducedExtensionMethodSymbol : MethodSymbol
/// </summary>
/// <param name="compilation">Compilation used to check constraints.
/// The latest language version is assumed if this is null.</param>
public static MethodSymbol Create(MethodSymbol method, TypeSymbol receiverType, CSharpCompilation compilation)
public static MethodSymbol Create(MethodSymbol method, TypeSymbol receiverType, CSharpCompilation compilation, out bool wasFullyInferred)
{
Debug.Assert(method.IsExtensionMethod && method.MethodKind != MethodKind.ReducedExtension);
Debug.Assert(method.ParameterCount > 0);
Debug.Assert((object)receiverType != null);

var useSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.DiscardedDependencies;

method = InferExtensionMethodTypeArguments(method, receiverType, compilation, ref useSiteInfo);
method = InferExtensionMethodTypeArguments(method, receiverType, compilation, ref useSiteInfo, out wasFullyInferred);
if ((object)method == null)
{
return null;
Expand Down Expand Up @@ -109,19 +109,22 @@ private ReducedExtensionMethodSymbol(MethodSymbol reducedFrom)
/// are not satisfied, the return value is null.
/// </summary>
/// <param name="compilation">Compilation used to check constraints. The latest language version is assumed if this is null.</param>
private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol method, TypeSymbol thisType, CSharpCompilation compilation, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol method, TypeSymbol thisType, CSharpCompilation compilation,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo, out bool wasFullyInferred)
{
Debug.Assert(method.IsExtensionMethod);
Debug.Assert((object)thisType != null);

if (!method.IsGenericMethod || method != method.ConstructedFrom)
{
wasFullyInferred = true;
return method;
}

// We never resolve extension methods on a dynamic receiver.
if (thisType.IsDynamic())
{
wasFullyInferred = false;
return null;
}

Expand Down Expand Up @@ -158,6 +161,7 @@ private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol metho

if (typeArgs.IsDefault)
{
wasFullyInferred = false;
return null;
}

Expand Down Expand Up @@ -215,12 +219,14 @@ private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol metho

if (!success)
{
wasFullyInferred = false;
return null;
}

// For the purpose of construction we use original type parameters in place of type arguments that we couldn't infer from the first argument.
ImmutableArray<TypeWithAnnotations> typeArgsForConstruct = typeArgs;
if (typeArgs.Any(static t => !t.HasType))
wasFullyInferred = !typeArgs.Any(static t => !t.HasType);
if (!wasFullyInferred)
{
typeArgsForConstruct = typeArgs.ZipAsArray(
method.TypeParameters,
Expand Down
Loading

0 comments on commit 4ed1916

Please sign in to comment.