Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a7b8337
ReplaceWithExtensionImplementation
jcouv Jul 15, 2025
77c106a
Address acceptOnlyMethods
jcouv Jul 15, 2025
a8a9da2
Follow-ups on unique signature
jcouv Jul 16, 2025
1fa8d49
Some missed comments for interceptors
jcouv Jul 16, 2025
5e37325
BuildArgumentsForErrorRecovery not reachable for extension properties
jcouv Jul 16, 2025
7a406cb
nullability
jcouv Jul 16, 2025
5324232
Semantic model follow-ups
jcouv Jul 16, 2025
8d46bd8
Operations
jcouv Jul 16, 2025
27934c4
See MakeDeconstructInvocationExpression line 661
jcouv Jul 16, 2025
550f190
Expected based on GetSemanticSymbols (called from GetMemberGroupForNode)
jcouv Jul 17, 2025
9d23ff2
Expression tree follow-up
jcouv Jul 17, 2025
4c91ee4
Method group with static/instance mismatch has expected behavior
jcouv Jul 17, 2025
77a8add
Base type check
jcouv Jul 17, 2025
c532cea
EnC follow-up
jcouv Jul 17, 2025
d928bbe
Order of events
jcouv Jul 17, 2025
b934bf5
TypeMap
jcouv Jul 17, 2025
7a434de
Dynamic invocation
jcouv Jul 17, 2025
08147a8
using directives
jcouv Jul 17, 2025
a0750b5
MemberNameSameAsType
jcouv Jul 17, 2025
454eaef
ExtractCastInvocation. Need some help
jcouv Jul 17, 2025
e62a6e7
Receiver requirements
jcouv Jul 17, 2025
9aa80f4
Tweaks
jcouv Jul 18, 2025
6a4b7d5
Strengthen LINQ Cast test
jcouv Jul 18, 2025
c2419b4
Address feedback
jcouv Jul 21, 2025
0cb34ee
Address feedback
jcouv Jul 22, 2025
01dbe85
Update comment
jcouv Jul 22, 2025
6e282c7
Merge remote-tracking branch 'dotnet/main' into extensions-followups
jcouv Jul 22, 2025
61b0198
Fix expected IL in net472 test
jcouv Jul 22, 2025
a0cc39d
Tag follow-up issue with used assemblies
jcouv Jul 23, 2025
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: 3 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ internal MethodInfo ReplaceWithExtensionImplementation(out bool wasError)
var setMethod = replace(SetMethod);
Symbol symbol = ReferenceEquals(Symbol, Method) && method is not null ? method : Symbol;

Debug.Assert(SetMethod?.GetIsNewExtensionMember() != true);
wasError = (Method is not null && method is null) || (SetMethod is not null && setMethod is null);

// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test in compound assignment (ie. "setMethod")
return new MethodInfo(symbol, method, setMethod);

static MethodSymbol? replace(MethodSymbol? method)
Expand All @@ -110,8 +110,7 @@ internal MethodInfo ReplaceWithExtensionImplementation(out bool wasError)
ConstructIfGeneric(method.ContainingType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics.Concat(method.TypeArgumentsWithAnnotations));
}

// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test this code path
return null;
throw ExceptionUtilities.Unreachable(); // we don't bind to extension methods with unsupported metadata
}
}

Expand Down Expand Up @@ -615,7 +614,7 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
{
var methodGroup = (BoundMethodGroup)expr;
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
diagnostics.Add(expr.Syntax, useSiteInfo);
Symbol otherSymbol = null;
Expand Down
138 changes: 61 additions & 77 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7970,7 +7970,7 @@ private BoundExpression MakeMemberAccessValue(BoundExpression expr, BindingDiagn
{
var methodGroup = (BoundMethodGroup)expr;
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
diagnostics.Add(expr.Syntax, useSiteInfo);
if (!expr.HasAnyErrors)
Expand Down Expand Up @@ -8851,60 +8851,6 @@ private bool AllowRefOmittedArguments(BoundExpression receiver)
}
#nullable disable

private void PopulateExtensionMethodsFromSingleBinder(
ExtensionScope scope,
MethodGroup methodGroup,
SyntaxNode node,
BoundExpression left,
string rightName,
ImmutableArray<TypeWithAnnotations> typeArgumentsWithAnnotations,
BindingDiagnosticBag diagnostics)
{
int arity;
if (typeArgumentsWithAnnotations.IsDefault)
{
arity = 0;
}
else
{
arity = typeArgumentsWithAnnotations.Length;
}

var lookupResult = LookupResult.GetInstance();
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
this.LookupExtensionMethods(lookupResult, scope, rightName, arity, ref useSiteInfo);
diagnostics.Add(node, useSiteInfo);

if (lookupResult.IsMultiViable)
{
Debug.Assert(lookupResult.Symbols.Any());
var members = ArrayBuilder<Symbol>.GetInstance();
bool wasError;
Symbol symbol = GetSymbolOrMethodOrPropertyGroup(lookupResult, node, rightName, arity, members, diagnostics, out wasError, qualifierOpt: null);
Debug.Assert((object)symbol == null);
Debug.Assert(members.Count > 0);
methodGroup.PopulateWithExtensionMethods(left, members, typeArgumentsWithAnnotations, lookupResult.Kind);
members.Free();
}

lookupResult.Free();
}

private void LookupExtensionMethods(LookupResult lookupResult, ExtensionScope scope, string rightName, int arity, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
LookupOptions options;
if (arity == 0)
{
options = LookupOptions.AllMethodsOnArityZero;
}
else
{
options = LookupOptions.Default;
}

this.LookupExtensionMethodsInSingleBinder(scope, lookupResult, rightName, arity, options, ref useSiteInfo);
}

protected BoundExpression BindFieldAccess(
SyntaxNode node,
BoundExpression receiver,
Expand Down Expand Up @@ -10520,7 +10466,8 @@ void makeCall(SyntaxNode syntax, BoundExpression receiver, MethodSymbol method,
indexerOrSliceAccess = BindMethodGroupInvocation(syntax, syntax, method.Name, boundMethodGroup, analyzedArguments,
diagnostics, queryClause: null, ignoreNormalFormIfHasValidParamsParameter: true, anyApplicableCandidates: out bool _,
disallowExpandedNonArrayParams: false,
acceptOnlyMethods: true).MakeCompilerGenerated(); // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test effect of acceptOnlyMethods value
acceptOnlyMethods: true) // acceptOnlyMethods is not relevant since we won't search extensions

@AlekseyTs AlekseyTs Jul 21, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since we won't search extensions

This fact is not obvious. Does this happen because boundMethodGroup.SearchExtensions is false? Consider adding an assert then. #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's because of BoundMethodGroupFlags.None above. The .SearchExtensions flag isn't set

.MakeCompilerGenerated();

analyzedArguments.Free();
}
Expand Down Expand Up @@ -10638,6 +10585,7 @@ internal MethodGroupResolution ResolveMethodGroup(
AnalyzedArguments analyzedArguments,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
OverloadResolution.Options options,
bool acceptOnlyMethods,
RefKind returnRefKind = default,
TypeSymbol returnType = null,
in CallingConventionInfo callingConventionInfo = default)
Expand All @@ -10649,7 +10597,7 @@ internal MethodGroupResolution ResolveMethodGroup(
return ResolveMethodGroup(
node, node.Syntax, node.Name, analyzedArguments, ref useSiteInfo,
options,
acceptOnlyMethods: true, // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Confirm this value is appropriate for all consumers of the enclosing method and test effect of this value for all of them
acceptOnlyMethods: acceptOnlyMethods,
returnRefKind: returnRefKind, returnType: returnType,
callingConventionInfo: callingConventionInfo);
}
Expand Down Expand Up @@ -10900,22 +10848,50 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
}
}

if (node.ReceiverOpt is not BoundTypeExpression && node.SearchExtensions)
if (node.SearchExtensions)
{
var receiver = node.ReceiverOpt!;
Debug.Assert(node.ReceiverOpt!.Type is not null); // extensions are only considered on member access

BoundExpression receiver = node.ReceiverOpt;
ImmutableArray<TypeWithAnnotations> typeArguments = node.TypeArgumentsOpt;
int arity = typeArguments.IsDefaultOrEmpty ? 0 : typeArguments.Length;
LookupOptions options = arity == 0 ? LookupOptions.AllMethodsOnArityZero : LookupOptions.Default;
var singleLookupResults = ArrayBuilder<SingleLookupResult>.GetInstance();
CompoundUseSiteInfo<AssemblySymbol> discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;

foreach (var scope in new ExtensionScopes(this))
{
methods.Clear();
var methodGroup = MethodGroup.GetInstance();
PopulateExtensionMethodsFromSingleBinder(scope, methodGroup, node.Syntax, receiver, node.Name, node.TypeArgumentsOpt, BindingDiagnosticBag.Discarded);
foreach (var m in methodGroup.Methods)
singleLookupResults.Clear();
scope.Binder.EnumerateAllExtensionMembersInSingleBinder(singleLookupResults, node.Name, arity, options, originalBinder: this, ref discardedUseSiteInfo, ref discardedUseSiteInfo);

foreach (SingleLookupResult singleLookupResult in singleLookupResults)
{
if (m.ReduceExtensionMethod(receiver.Type, Compilation) is { } reduced)
Symbol extensionMember = singleLookupResult.Symbol;
if (IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(receiver, extensionMember))
{
methods.Add(reduced);
// Remove static/instance mismatches
continue;
}

// Note: we only care about methods. If the expression resolved to a non-method extension member, we wouldn't get here to compute the function type for the expression.
if (extensionMember is MethodSymbol m)
{
if (m.GetIsNewExtensionMember())
{
// Note: new extension methods are subject to more stringent checks
var substituted = (MethodSymbol?)extensionMember.GetReducedAndFilteredSymbol(typeArguments, receiver.Type, Compilation, checkFullyInferred: true);
if (substituted is not null)
{
methods.Add(substituted);
}
}
else if (m.ReduceExtensionMethod(receiver.Type, Compilation) is { } reduced)
{
methods.Add(reduced);
}
}
}
methodGroup.Free();

if (methods.Count == 0)
{
Expand All @@ -10926,6 +10902,7 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
{
methods.Free();
useParams = false;
singleLookupResults.Free();
return null;
}

Expand All @@ -10936,6 +10913,7 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
{
methods.Free();
useParams = false;
singleLookupResults.Free();
return null;
}

Expand All @@ -10948,10 +10926,13 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
{
methods.Free();
useParams = false;
singleLookupResults.Free();
return null;
}
}
}

singleLookupResults.Free();
}

methods.Free();
Expand Down Expand Up @@ -10989,6 +10970,17 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
}
}

private static bool IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(BoundExpression receiver, Symbol extensionMember)
{
bool memberCountsAsStatic = extensionMember is MethodSymbol { IsExtensionMethod: true } ? false : extensionMember.IsStatic;

@AlekseyTs AlekseyTs Jul 21, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bool memberCountsAsStatic = extensionMember is MethodSymbol { IsExtensionMethod: true } ? false : extensionMember.IsStatic;

This logic is too special for a method with such "innocent" name. It looks like the method is meant to be used only under specific conditions. Consider renaming to avoid an accidental misuse of the method. #Closed

return receiver switch
{
BoundTypeOrValueExpression => false,
BoundTypeExpression => !memberCountsAsStatic,
_ => memberCountsAsStatic,
};
}

/// <summary>
/// For C# 13 onwards, returns one of the methods from the method group if all instance methods, or extension methods
/// in the nearest scope, have the same signature ignoring parameter names and custom modifiers.
Expand Down Expand Up @@ -11088,23 +11080,15 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
var methods = ArrayBuilder<MethodSymbol>.GetInstance(capacity: singleLookupResults.Count);
foreach (SingleLookupResult singleLookupResult in singleLookupResults)
{
// Remove static/instance mismatches
Symbol extensionMember = singleLookupResult.Symbol;
bool memberCountsAsStatic = extensionMember is MethodSymbol { IsExtensionMethod: true } ? false : extensionMember.IsStatic;
switch (node.ReceiverOpt)
if (IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(receiver, extensionMember))
{
case BoundTypeOrValueExpression:
break;
case BoundTypeExpression:
if (!memberCountsAsStatic) continue;
break;
default:
if (memberCountsAsStatic) continue;
break;
// Remove static/instance mismatches
continue;
}

// Note: we only care about methods since we're already decided this is a method group (ie. not resolving to some other kind of extension member)
if (extensionMember is MethodSymbol method)
if (extensionMember is MethodSymbol)
{
var substituted = (MethodSymbol?)extensionMember.GetReducedAndFilteredSymbol(typeArguments, receiver.Type, Compilation, checkFullyInferred: true);
if (substituted is not null)
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2069,7 +2069,7 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
var parameterListList = ArrayBuilder<ImmutableArray<ParameterSymbol>>.GetInstance();
foreach (var p in properties)
{
// Tracked by https://github.com/dotnet/roslyn/issues/76130: Revisit this with new extensions
Debug.Assert(!p.GetIsNewExtensionMember());
if (p.ParameterCount > 0)
{
parameterListList.Add(p.Parameters);
Expand Down Expand Up @@ -2353,7 +2353,7 @@ private void EnsureNameofExpressionSymbols(BoundMethodGroup methodGroup, Binding
{
// Check that the method group contains something applicable. Otherwise error.
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
var resolution = ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
var resolution = ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));

diagnostics.Add(methodGroup.Syntax, useSiteInfo);
Expand Down
22 changes: 0 additions & 22 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -525,28 +525,6 @@ private static void LookupMembersInNamespace(LookupResult result, NamespaceSymbo
}
}

// Tracked by https://github.com/dotnet/roslyn/issues/76130 : we should be able to remove this method once all the callers are updated to account for new extension members
/// <summary>
/// Lookup extension methods by name and arity in the given binder and
/// check viability in this binder. The lookup is performed on a single
/// binder because extension method search stops at the first applicable
/// method group from the nearest enclosing namespace.
/// </summary>
private void LookupExtensionMethodsInSingleBinder(ExtensionScope scope, LookupResult result, string name, int arity, LookupOptions options, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var methods = ArrayBuilder<MethodSymbol>.GetInstance();
var binder = scope.Binder;
binder.GetCandidateExtensionMethods(methods, name, arity, options, this);

foreach (var method in methods)
{
SingleLookupResult resultOfThisMember = this.CheckViability(method, arity, options, null, diagnose: true, useSiteInfo: ref useSiteInfo);
result.MergeEqual(resultOfThisMember);
}

methods.Free();
}

#region "AttributeTypeLookup"

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Query.cs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ private void ReduceFrom(FromClauseSyntax from, QueryTranslationState state, Bind

private static BoundExpression? ExtractCastInvocation(BoundCall invocation)
{
int index = invocation.InvokedAsExtensionMethod ? 1 : 0; // Tracked by https://github.com/dotnet/roslyn/issues/76130: Add test coverage for his code path

@jcouv jcouv Jul 18, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

📝 The test I added definitely covers this (debugged) but I've not yet been able to make the test sensitive to bad intentionally logic here... I'll try some more Test now shows that we have correct behavior here :-) #Closed

int index = invocation.InvokedAsExtensionMethod ? 1 : 0;
var c1 = invocation.Arguments[index] as BoundConversion;
var l1 = c1 != null ? c1.Operand as BoundLambda : null;
var r1 = l1 != null ? l1.Body.Statements[0] as BoundReturnStatement : null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ private static MethodGroupResolution ResolveDelegateOrFunctionPointerMethodGroup
resolution = binder.ResolveMethodGroup(source, analyzedArguments, useSiteInfo: ref useSiteInfo,
options: OverloadResolution.Options.InferWithDynamic | OverloadResolution.Options.IsMethodGroupConversion |
(isFunctionPointer ? OverloadResolution.Options.IsFunctionPointerResolution : OverloadResolution.Options.None),
returnRefKind: delegateInvokeMethodOpt.RefKind, returnType: delegateInvokeMethodOpt.ReturnType,
acceptOnlyMethods: true, returnRefKind: delegateInvokeMethodOpt.RefKind, returnType: delegateInvokeMethodOpt.ReturnType,
callingConventionInfo: callingConventionInfo);
analyzedArguments.Free();
}
else
{
resolution = binder.ResolveMethodGroup(source, analyzedArguments: null, ref useSiteInfo, options: OverloadResolution.Options.IsMethodGroupConversion);
resolution = binder.ResolveMethodGroup(source, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.IsMethodGroupConversion, acceptOnlyMethods: true);
}

Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,7 @@ private TypeWithAnnotations MethodGroupReturnType(
var resolution = binder.ResolveMethodGroup(source, analyzedArguments, useSiteInfo: ref useSiteInfo,
options: OverloadResolution.Options.IsMethodGroupConversion |
(isFunctionPointerResolution ? OverloadResolution.Options.IsFunctionPointerResolution : OverloadResolution.Options.None),
returnRefKind: delegateRefKind,
acceptOnlyMethods: true, returnRefKind: delegateRefKind,
// Since we are trying to infer the return type, it is not an input to resolving the method group
returnType: null,
callingConventionInfo: in callingConventionInfo);
Expand Down
Loading