Skip to content
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

Jump to bottom

Resolve extension members in all non-invocation contexts #73239

Merged
merged 13 commits into from
May 2, 2024
Merged

Resolve extension members in all non-invocation contexts #73239

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bba7662
Resolve extension members in all non-invocation contexts
jcouv Apr 24, 2024
e13078b
Address feedback
jcouv Apr 30, 2024
a5d2a0b
Tweak
jcouv May 1, 2024
7ae3ae9
Tweak a test
jcouv May 1, 2024
f4f96c6
Address feedback
jcouv May 1, 2024
f33ffce
Address feedback
jcouv May 1, 2024
db623f9
Fix collection scenario and test
jcouv May 1, 2024
0b41777
Remove PROTOTYPE comment
jcouv May 1, 2024
b523dc2
Address feedback
jcouv May 2, 2024
a942cc4
Cosmetic
jcouv May 2, 2024
28770c1
One more test
jcouv May 2, 2024
e5ac37e
Fix a test
jcouv May 2, 2024
5770a19
Tweak comment
jcouv May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 1 addition & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,11 +406,6 @@ private BoundIndexerAccess BindIndexerDefaultArgumentsAndParamsCollection(BoundI
/// </summary>
private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind, BindingDiagnosticBag diagnostics)
{
if (RequiresAssignableVariable(valueKind))
{
expr = ResolveToExtensionMemberIfPossible(expr, diagnostics);
}

switch (expr.Kind)
{
case BoundKind.PropertyGroup:
Expand Down Expand Up @@ -482,7 +477,7 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
diagnostics.Add(expr.Syntax, useSiteInfo);
Symbol otherSymbol = null;
bool resolvedToUnusableSymbol = resolution.MethodGroup == null && !resolution.IsExtensionMember(out _);
bool resolvedToUnusableSymbol = resolution.MethodGroup == null && !resolution.IsNonMethodExtensionMember(out _);
if (!expr.HasAnyErrors) diagnostics.AddRange(resolution.Diagnostics); // Suppress cascading.
hasResolutionErrors = resolution.HasAnyErrors;
if (hasResolutionErrors)
Expand Down
12 changes: 1 addition & 11 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ internal partial class Binder

Debug.Assert(result is BoundConversion
|| (conversion.IsIdentity && ((object)result == source)
|| (conversion.IsExtensionMemberConversion(out _, out var nestedConversion) && nestedConversion.IsIdentity)
|| source.NeedsToBeConverted())
|| hasErrors);

Expand Down Expand Up @@ -135,15 +134,6 @@ static bool filterConversion(Conversion conversion)
return CreateMethodGroupConversion(syntax, source, conversion, isCast: isCast, conversionGroupOpt, destination, diagnostics);
}

if (conversion.IsExtensionMemberConversion(out Symbol? extensionMember, out Conversion nestedConversion))
{
var methodGroup = (BoundMethodGroup)source;
source = GetExtensionMemberAccess(syntax, methodGroup.ReceiverOpt, extensionMember,
methodGroup.TypeArgumentsSyntax, methodGroup.TypeArgumentsOpt, diagnostics);

return CreateConversion(syntax, source, nestedConversion, isCast, conversionGroupOpt, destination, diagnostics);
}

// Obsolete diagnostics for method group are reported as part of creating the method group conversion.
reportUseSiteDiagnostics(syntax, conversion, source, destination, diagnostics);

Expand Down Expand Up @@ -529,7 +519,7 @@ void checkConstraintLanguageVersionAndRuntimeSupportForConversion(SyntaxNode syn
return BindFieldAccess(syntax, receiver, fieldSymbol, diagnostics, LookupResultKind.Viable, indexed: false, hasErrors: false);

case NamedTypeSymbol namedTypeSymbol:
bool wasError = false;
bool wasError = namedTypeSymbol.IsErrorType();

if (!typeArgumentsOpt.IsDefault)
{
Expand Down
36 changes: 13 additions & 23 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ internal BoundExpression BindToTypeForErrorRecovery(BoundExpression expression,
/// </summary>
internal BoundExpression BindToNaturalType(BoundExpression expression, BindingDiagnosticBag diagnostics, bool reportNoTargetType = true)
{
expression = ResolveToExtensionMemberIfPossible(expression, diagnostics);
Debug.Assert(object.ReferenceEquals(ResolveToNonMethodExtensionMemberIfPossible(expression, BindingDiagnosticBag.Discarded), expression),
"An empty method group that resolves to a non-method extension member should have been resolved by now");

if (!expression.NeedsToBeConverted())
return expression;
Expand Down Expand Up @@ -408,24 +409,6 @@ internal BoundExpression BindToNaturalType(BoundExpression expression, BindingDi
return result?.WithWasConverted();
}

#nullable enable
private BoundExpression BindToExtensionMemberOrInferredDelegateType(BoundMethodGroup methodGroup, BindingDiagnosticBag diagnostics)
{
// PROTOTYPE test use-site diagnostics
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
MethodGroupResolution resolution = ResolveMethodGroup(methodGroup, analyzedArguments: null, ref useSiteInfo, options: OverloadResolution.Options.None);
diagnostics.Add(methodGroup.Syntax, useSiteInfo);
if (resolution.IsExtensionMember(out Symbol? extensionMember))
{
return GetExtensionMemberAccess(methodGroup.Syntax, methodGroup.ReceiverOpt, extensionMember,
methodGroup.TypeArgumentsSyntax, methodGroup.TypeArgumentsOpt, diagnostics);
}

// We have a method group so bind to an inferred delegate type
return BindToInferredDelegateType(methodGroup, diagnostics);
}
#nullable disable

private BoundExpression BindToInferredDelegateType(BoundExpression expr, BindingDiagnosticBag diagnostics)
{
Debug.Assert(expr.Kind is BoundKind.UnboundLambda or BoundKind.MethodGroup);
Expand Down Expand Up @@ -598,6 +581,13 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, BindingDia

BoundExpression result = bindExpressionInternal(node, diagnostics, invoked, indexed);

// Only invocation expressions need to resolve extension members as invoked (ie. filtering out non-invocable members)
// All other contexts can go ahead and resolve to a non-method extension member
if (!invoked)
{
result = ResolveToNonMethodExtensionMemberIfPossible(result, diagnostics);
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 30, 2024 •
edited

Choose a reason for hiding this comment

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

ResolveToNonMethodExtensionMemberIfPossible

Is the name somewhat confusing? We may still resolve to methods, right? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I didn't quite understand the question. ResolveToNonMethodExtensionMemberIfPossible only returns non-methods.

}

if (IsEarlyAttributeBinder && result.Kind == BoundKind.MethodGroup && (!IsInsideNameof || EnclosingNameofArgument != node))
{
return BadExpression(node, LookupResultKind.NotAValue);
Expand Down Expand Up @@ -7499,7 +7489,7 @@ private BoundExpression MakeMemberAccessValue(BoundExpression expr, BindingDiagn
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
diagnostics.Add(expr.Syntax, useSiteInfo);

if (resolution.IsExtensionMember(out Symbol extensionMember))
if (resolution.IsNonMethodExtensionMember(out Symbol extensionMember))
{
diagnostics.AddRange(resolution.Diagnostics);
return GetExtensionMemberAccess(methodGroup.Syntax, methodGroup.ReceiverOpt, extensionMember,
Expand Down Expand Up @@ -7530,7 +7520,7 @@ private BoundExpression MakeMemberAccessValue(BoundExpression expr, BindingDiagn
}
}

private BoundExpression ResolveToExtensionMemberIfPossible(BoundExpression expr, BindingDiagnosticBag diagnostics)
internal BoundExpression ResolveToNonMethodExtensionMemberIfPossible(BoundExpression expr, BindingDiagnosticBag diagnostics)
{
switch (expr.Kind)
{
Expand All @@ -7541,7 +7531,7 @@ private BoundExpression ResolveToExtensionMemberIfPossible(BoundExpression expr,
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, ref useSiteInfo, options: OverloadResolution.Options.None);
diagnostics.Add(expr.Syntax, useSiteInfo);

if (resolution.IsExtensionMember(out Symbol extensionMember))
if (resolution.IsNonMethodExtensionMember(out Symbol extensionMember))
{
diagnostics.AddRange(resolution.Diagnostics);
resolution.Free();
Expand Down Expand Up @@ -8023,7 +8013,7 @@ private static void CombineExtensionMethodArguments(BoundExpression receiver, An
options, returnRefKind, returnType, withDependencies, scope, in callingConvention,
out MethodGroupResolution extensionResult))
{
if (extensionResult.IsExtensionMember(out _))
if (extensionResult.IsNonMethodExtensionMember(out _))
{
return extensionResult;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,14 +721,15 @@ private bool HasApplicableMemberWithPossiblyExpandedNonArrayParamsCollection<TMe
(analyzedArguments.HasDynamicArgument ? OverloadResolution.Options.DynamicResolution : OverloadResolution.Options.None));
diagnostics.Add(expression, useSiteInfo);

if (resolution.IsExtensionMember(out Symbol extensionMember))
if (resolution.IsNonMethodExtensionMember(out Symbol extensionMember))
Copy link
Contributor

@AlekseyTs AlekseyTs May 1, 2024 •
edited

Choose a reason for hiding this comment

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

if (resolution.IsNonMethodExtensionMember(out Symbol extensionMember))

Is this if statement still necessary? Is its body still reachable? Can you give an example of a scenario? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can still get here. Below is an example.
When we have an invocation, we delay the resolution of extensions when binding a member access. We resolve the extensions when binding the invocation (ie. here). So we may find that our receiver is not a method group, but instead an invocable member (for example a delegate type field).
In such case, we bind that member access to the invocable member and invoke it with the given arguments.

    public void ExtensionInvocation_OnlyDelegateFieldExists()
    {
        // Invocable fields are considered during extension invocation
        var source = """
E.Field = (i) => { System.Console.Write($"ran({i})"); };
C.Field(42);

class C { }

delegate void D(int i);

implicit extension E for C
{
    public static D Field;
}
""";

{
diagnostics.AddRange(resolution.Diagnostics);
var extensionMemberAccess = GetExtensionMemberAccess(expression, methodGroup.ReceiverOpt, extensionMember,
methodGroup.TypeArgumentsSyntax, methodGroup.TypeArgumentsOpt, diagnostics);

Debug.Assert(extensionMemberAccess.Kind != BoundKind.MethodGroup);

extensionMemberAccess = CheckValue(extensionMemberAccess, BindValueKind.RValue, diagnostics);
var extensionMemberInvocation = BindInvocationExpression(syntax, expression, methodName, extensionMemberAccess, analyzedArguments, diagnostics);
anyApplicableCandidates = !extensionMemberInvocation.HasAnyErrors;
return extensionMemberInvocation;
Expand Down Expand Up @@ -2372,7 +2373,6 @@ private BoundExpression BindNameofOperatorInternal(InvocationExpressionSyntax no
CheckFeatureAvailability(node, MessageID.IDS_FeatureNameof, diagnostics);
var argument = node.ArgumentList.Arguments[0].Expression;
var boundArgument = BindExpression(argument, diagnostics);
boundArgument = ResolveToExtensionMemberIfPossible(boundArgument, diagnostics);

bool syntaxIsOk = CheckSyntaxForNameofArgument(argument, out string name, boundArgument.HasAnyErrors ? BindingDiagnosticBag.Discarded : diagnostics);
if (!boundArgument.HasAnyErrors && syntaxIsOk && boundArgument.Kind == BoundKind.MethodGroup)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ protected BoundExpression BindInferredVariableInitializer(BindingDiagnosticBag d
BoundExpression expression = value.Kind switch
{
BoundKind.UnboundLambda => BindToInferredDelegateType(value, diagnostics),
BoundKind.MethodGroup => BindToExtensionMemberOrInferredDelegateType((BoundMethodGroup)value, diagnostics),
BoundKind.MethodGroup => BindToInferredDelegateType(value, diagnostics),
_ => BindToNaturalType(value, diagnostics)
};

Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Binder/MethodGroupResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,8 @@ public bool IsExtensionMethodGroup
#nullable enable
/// <summary>
/// Indicates that we have a viable result that is a non-method extension member.
/// PROTOTYPE consider renaming the method to IsNonMethodExtensionMember
/// </summary>
public bool IsExtensionMember([NotNullWhen(true)] out Symbol? extensionMember)
public bool IsNonMethodExtensionMember([NotNullWhen(true)] out Symbol? extensionMember)
{
bool isExtensionMember = ResultKind == LookupResultKind.Viable && MethodGroup is null;
extensionMember = isExtensionMember ? OtherSymbol : null;
Expand Down
42 changes: 0 additions & 42 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,6 @@ internal DeconstructionUncommonData(DeconstructMethodInfo deconstructMethodInfoO
internal readonly ImmutableArray<(BoundValuePlaceholder? placeholder, BoundExpression? conversion)> DeconstructConversionInfo;
}

private class ExtensionMemberUncommonData : NestedUncommonData
{
internal readonly Symbol ExtensionMember;

internal ExtensionMemberUncommonData(Symbol extensionMember, ImmutableArray<Conversion> nestedConversions)
: base(nestedConversions)
{
Debug.Assert(extensionMember is not null);
Debug.Assert(nestedConversions.Length == 1);
Debug.Assert(nestedConversions[0].Kind != ConversionKind.ExtensionMember);

ExtensionMember = extensionMember;
}
}

private sealed class CollectionExpressionUncommonData : NestedUncommonData
{
internal CollectionExpressionUncommonData(
Expand Down Expand Up @@ -186,13 +171,6 @@ internal Conversion(ConversionKind kind, MethodSymbol conversionMethod, bool isE
conversionMethod: conversionMethod);
}

// For extension member conversion
internal Conversion(Symbol extensionMember, Conversion nestedConversion)
{
this._kind = ConversionKind.ExtensionMember;
_uncommonData = new ExtensionMemberUncommonData(extensionMember, nestedConversions: ImmutableArray.Create(nestedConversion));
}

internal Conversion(ConversionKind kind, ImmutableArray<Conversion> nestedConversions)
{
this._kind = kind;
Expand Down Expand Up @@ -563,26 +541,6 @@ internal DeconstructMethodInfo DeconstructionInfo
}
}

internal bool IsExtensionMemberConversion([NotNullWhen(true)] out Symbol? extensionMember, out Conversion nestedConversion)
{
if (Kind != ConversionKind.ExtensionMember)
{
extensionMember = null;
nestedConversion = default;
return false;
}

var uncommonData = (ExtensionMemberUncommonData?)_uncommonData;

Debug.Assert(uncommonData is not null);
extensionMember = uncommonData.ExtensionMember;

Debug.Assert(uncommonData._nestedConversionsOpt.Length == 1);
nestedConversion = uncommonData._nestedConversionsOpt[0];

return true;
}

internal CollectionExpressionTypeKind GetCollectionExpressionTypeKind(out TypeSymbol? elementType, out MethodSymbol? constructor, out bool isExpanded)
{
if (_uncommonData is CollectionExpressionUncommonData collectionExpressionData)
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,5 @@ internal enum ConversionKind : byte

InterpolatedStringHandler, // A conversion from an interpolated string literal to a type attributed with InterpolatedStringBuilderAttribute
InlineArray, // A conversion from an inline array to Span/ReadOnlySpan
ExtensionMember, // The ExtensionMember conversion is not part of the language, it is an implementation detail
}
}
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionKindExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public static bool IsImplicitConversion(this ConversionKind conversionKind)
case ObjectCreation:
case InlineArray:
case CollectionExpression:
case ExtensionMember:
return true;

case ExplicitNumeric:
Expand Down
38 changes: 1 addition & 37 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,6 @@ public override Conversion GetMethodGroupDelegateConversion(BoundMethodGroup sou
// Must be a bona fide delegate type, not an expression tree type.
if (!destination.IsDelegateType())
{
if (tryGetExtensionMemberConversion(source, destination, ref useSiteInfo, out var extensionMemberConversion))
{
return extensionMemberConversion;
}

return Conversion.NoConversion;
}

Expand Down Expand Up @@ -107,44 +102,13 @@ public override Conversion GetMethodGroupDelegateConversion(BoundMethodGroup sou
}

var resolution = ResolveDelegateOrFunctionPointerMethodGroup(_binder, source, methodSymbol, isFunctionPointer, callingConventionInfo, ref useSiteInfo);
if (resolution.IsExtensionMember(out var extensionMember))
{
var nestedConversion = ClassifyConversionFromExpressionType(extensionMember.GetTypeOrReturnType().Type, destination, isChecked: false, ref useSiteInfo);
if (nestedConversion.Kind != ConversionKind.NoConversion)
{
return new Conversion(extensionMember, nestedConversion);
}
}
Debug.Assert(!resolution.IsNonMethodExtensionMember(extensionMember: out _));

var conversion = (resolution.IsEmpty || resolution.HasAnyErrors) ?
Conversion.NoConversion :
ToConversion(resolution.OverloadResolutionResult, resolution.MethodGroup, methodSymbol.ParameterCount);
resolution.Free();
return conversion;

bool tryGetExtensionMemberConversion(BoundMethodGroup source, TypeSymbol destination,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo, out Conversion extensionMemberConversion)
{
if (source is not { Methods: [], SearchExtensionMethods: true })
{
extensionMemberConversion = default;
return false;
}

MethodGroupResolution resolution = _binder.ResolveMethodGroup(source, analyzedArguments: null, ref useSiteInfo, options: OverloadResolution.Options.None);
if (resolution.IsExtensionMember(out Symbol? extensionMember) && extensionMember is not NamedTypeSymbol)
{
var nestedConversion = ClassifyConversionFromExpressionType(extensionMember.GetTypeOrReturnType().Type, destination, isChecked: false, ref useSiteInfo);
if (nestedConversion.Kind != ConversionKind.NoConversion)
{
extensionMemberConversion = new Conversion(extensionMember, nestedConversion);
return true;
}
}

extensionMemberConversion = default;
return false;
}
}
#nullable disable

Expand Down