Skip to content

Commit c936508

Browse files
authored
Extensions: Review/adjust call sites of MethodSymbol.IsExtensionMethod API (#77820)
1 parent cf3f089 commit c936508

14 files changed

+448
-57
lines changed

src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1708,7 +1708,7 @@ internal bool TryGetCollectionIterationType(SyntaxNode syntax, TypeSymbol collec
17081708
out iterationType,
17091709
builder: out var builder);
17101710
// Collection expression target types require instance method GetEnumerator.
1711-
if (result && builder.ViaExtensionMethod)
1711+
if (result && builder.ViaExtensionMethod) // PROTOTYPE: Add test coverage for new extensions
17121712
{
17131713
iterationType = default;
17141714
return false;

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ private BoundExpression MakeDeconstructInvocationExpression(
683683
// This prevents, for example, an unused params parameter after the out parameters.
684684
var deconstructMethod = ((BoundCall)result).Method;
685685
var parameters = deconstructMethod.Parameters;
686-
for (int i = (deconstructMethod.IsExtensionMethod ? 1 : 0); i < parameters.Length; i++)
686+
for (int i = (deconstructMethod.IsExtensionMethod ? 1 : 0); i < parameters.Length; i++) // PROTOTYPE: Test this code path with new extensions
687687
{
688688
if (parameters[i].RefKind != RefKind.Out)
689689
{

src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,7 @@ deconstructMethod is null &&
10581058
if (deconstructMethod is null)
10591059
hasErrors = true;
10601060

1061-
int skippedExtensionParameters = deconstructMethod?.IsExtensionMethod == true ? 1 : 0;
1061+
int skippedExtensionParameters = deconstructMethod?.IsExtensionMethod == true ? 1 : 0; // PROTOTYPE: Test this code path with new extensions
10621062
for (int i = 0; i < node.Subpatterns.Count; i++)
10631063
{
10641064
var subPattern = node.Subpatterns[i];

src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs

Lines changed: 74 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -236,12 +236,20 @@ private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagno
236236
{
237237
originalBinder.CheckImplicitThisCopyInReadOnlyMember(collectionExpr, getEnumeratorMethod, diagnostics);
238238

239-
if (getEnumeratorMethod.IsExtensionMethod && !hasErrors)
239+
if (!hasErrors)
240240
{
241-
var messageId = IsAsync ? MessageID.IDS_FeatureExtensionGetAsyncEnumerator : MessageID.IDS_FeatureExtensionGetEnumerator;
242-
messageId.CheckFeatureAvailability(diagnostics, Compilation, collectionExpr.Syntax.Location);
241+
if (getEnumeratorMethod.IsExtensionMethod)
242+
{
243+
var messageId = IsAsync ? MessageID.IDS_FeatureExtensionGetAsyncEnumerator : MessageID.IDS_FeatureExtensionGetEnumerator;
244+
messageId.CheckFeatureAvailability(diagnostics, Compilation, collectionExpr.Syntax.Location);
243245

244-
if (getEnumeratorMethod.ParameterRefKinds is { IsDefault: false } refKinds && refKinds[0] == RefKind.Ref)
246+
if (getEnumeratorMethod.ParameterRefKinds is { IsDefault: false } refKinds && refKinds[0] == RefKind.Ref)
247+
{
248+
Error(diagnostics, ErrorCode.ERR_RefLvalueExpected, collectionExpr.Syntax);
249+
hasErrors = true;
250+
}
251+
}
252+
else if (getEnumeratorMethod.GetIsNewExtensionMember() && getEnumeratorMethod.ContainingType.ExtensionParameter.RefKind == RefKind.Ref) // PROTOTYPE: add test coverage for 'ref readonly' and 'in'
245253
{
246254
Error(diagnostics, ErrorCode.ERR_RefLvalueExpected, collectionExpr.Syntax);
247255
hasErrors = true;
@@ -570,7 +578,8 @@ private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagno
570578
(collectionConversionClassification.IsImplicit &&
571579
(IsIEnumerable(builder.CollectionType) ||
572580
IsIEnumerableT(builder.CollectionType.OriginalDefinition, IsAsync, Compilation) ||
573-
builder.GetEnumeratorInfo.Method.IsExtensionMethod)) ||
581+
builder.GetEnumeratorInfo.Method.IsExtensionMethod ||
582+
builder.GetEnumeratorInfo.Method.GetIsNewExtensionMember())) ||
574583
// For compat behavior, we can enumerate over System.String even if it's not IEnumerable. That will
575584
// result in an explicit reference conversion in the bound nodes, but that conversion won't be emitted.
576585
(collectionConversionClassification.Kind == ConversionKind.ExplicitReference && collectionExpr.Type.SpecialType == SpecialType.System_String));
@@ -861,9 +870,9 @@ private EnumeratorResult GetEnumeratorInfoCore(SyntaxNode syntax, SyntaxNode col
861870

862871
#if DEBUG
863872
Debug.Assert(span == originalSpan);
864-
Debug.Assert(!builder.ViaExtensionMethod || builder.GetEnumeratorInfo.Method.IsExtensionMethod);
873+
Debug.Assert(!builder.ViaExtensionMethod || builder.GetEnumeratorInfo.Method.IsExtensionMethod || builder.GetEnumeratorInfo.Method.GetIsNewExtensionMember());
865874
#endif
866-
if (!builder.ViaExtensionMethod &&
875+
if (!builder.ViaExtensionMethod && // PROTOTYPE: Add test coverage for new extensions
867876
((result is EnumeratorResult.Succeeded && builder.ElementTypeWithAnnotations.Equals(elementField.TypeWithAnnotations, TypeCompareKind.AllIgnoreOptions) &&
868877
builder.CurrentPropertyGetter?.RefKind == (wellKnownSpan == WellKnownType.System_ReadOnlySpan_T ? RefKind.RefReadOnly : RefKind.Ref)) ||
869878
result is EnumeratorResult.FailedAndReported))
@@ -908,7 +917,7 @@ private EnumeratorResult GetEnumeratorInfoCore(SyntaxNode syntax, SyntaxNode col
908917
#if DEBUG
909918
Debug.Assert(collectionExpr == originalCollectionExpr ||
910919
(originalCollectionExpr.Type?.IsNullableType() == true && originalCollectionExpr.Type.StrippedType().Equals(collectionExpr.Type, TypeCompareKind.AllIgnoreOptions)));
911-
Debug.Assert(!builder.ViaExtensionMethod || builder.GetEnumeratorInfo.Method.IsExtensionMethod);
920+
Debug.Assert(!builder.ViaExtensionMethod || builder.GetEnumeratorInfo.Method.IsExtensionMethod || builder.GetEnumeratorInfo.Method.GetIsNewExtensionMember());
912921
#endif
913922

914923
return result;
@@ -1019,12 +1028,26 @@ EnumeratorResult createPatternBasedEnumeratorResult(ref ForEachEnumeratorInfo.Bu
10191028
{
10201029
Debug.Assert((object)builder.GetEnumeratorInfo != null);
10211030

1022-
Debug.Assert(!(viaExtensionMethod && builder.GetEnumeratorInfo.Method.Parameters.IsDefaultOrEmpty));
1031+
Debug.Assert(!(viaExtensionMethod && builder.GetEnumeratorInfo.Method.IsExtensionMethod && builder.GetEnumeratorInfo.Method.Parameters.IsDefaultOrEmpty));
1032+
Debug.Assert(!(viaExtensionMethod && !builder.GetEnumeratorInfo.Method.IsExtensionMethod && !builder.GetEnumeratorInfo.Method.GetIsNewExtensionMember()));
10231033

10241034
builder.ViaExtensionMethod = viaExtensionMethod;
1025-
builder.CollectionType = viaExtensionMethod
1026-
? builder.GetEnumeratorInfo.Method.Parameters[0].Type
1027-
: collectionExpr.Type;
1035+
1036+
if (viaExtensionMethod)
1037+
{
1038+
if (builder.GetEnumeratorInfo.Method.IsExtensionMethod)
1039+
{
1040+
builder.CollectionType = builder.GetEnumeratorInfo.Method.Parameters[0].Type;
1041+
}
1042+
else
1043+
{
1044+
builder.CollectionType = builder.GetEnumeratorInfo.Method.ContainingType.ExtensionParameter.Type;
1045+
}
1046+
}
1047+
else
1048+
{
1049+
builder.CollectionType = collectionExpr.Type;
1050+
}
10281051

10291052
if (SatisfiesForEachPattern(syntax, collectionSyntax, ref builder, isAsync, diagnostics))
10301053
{
@@ -1200,7 +1223,7 @@ private void GetDisposalInfoForEnumerator(SyntaxNode syntax, ref ForEachEnumerat
12001223
MethodSymbol patternDisposeMethod = TryFindDisposePatternMethod(receiver, syntax, isAsync, patternDiagnostics, out bool expanded);
12011224
if (patternDisposeMethod is object)
12021225
{
1203-
Debug.Assert(!patternDisposeMethod.IsExtensionMethod);
1226+
Debug.Assert(!patternDisposeMethod.IsExtensionMethod && !patternDisposeMethod.GetIsNewExtensionMember());
12041227
Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty ||
12051228
patternDisposeMethod.ParameterRefKinds.All(static refKind => refKind is RefKind.None or RefKind.In or RefKind.RefReadOnlyParameter));
12061229

@@ -1522,6 +1545,8 @@ private MethodArgumentInfo FindForEachPatternMethodViaExtension(SyntaxNode synta
15221545
{
15231546
var result = overloadResolutionResult.ValidResult.Member;
15241547

1548+
Debug.Assert(result.IsExtensionMethod || result.GetIsNewExtensionMember());
1549+
15251550
if (result.CallsAreOmitted(syntax.SyntaxTree))
15261551
{
15271552
// Calls to this method are omitted in the current syntax tree, i.e it is either a partial method with no implementation part OR a conditional method whose condition is not true in this source file.
@@ -1531,28 +1556,44 @@ private MethodArgumentInfo FindForEachPatternMethodViaExtension(SyntaxNode synta
15311556
return null;
15321557
}
15331558

1534-
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
1535-
var collectionConversion = this.Conversions.ClassifyConversionFromExpression(collectionExpr, result.Parameters[0].Type, isChecked: CheckOverflowAtRuntime, ref useSiteInfo);
1536-
diagnostics.Add(syntax, useSiteInfo);
1559+
MethodArgumentInfo info;
1560+
bool expanded = overloadResolutionResult.ValidResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm;
15371561

1538-
// Unconditionally convert here, to match what we set the ConvertedExpression to in the main BoundForEachStatement node.
1539-
Debug.Assert(!collectionConversion.IsUserDefined);
1540-
collectionExpr = new BoundConversion(
1541-
collectionExpr.Syntax,
1542-
collectionExpr,
1543-
collectionConversion,
1544-
@checked: CheckOverflowAtRuntime,
1545-
explicitCastInCode: false,
1546-
conversionGroupOpt: null,
1547-
ConstantValue.NotAvailable,
1548-
result.Parameters[0].Type);
1562+
if (result.IsExtensionMethod)
1563+
{
1564+
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
1565+
var collectionConversion = this.Conversions.ClassifyConversionFromExpression(collectionExpr, result.Parameters[0].Type, isChecked: CheckOverflowAtRuntime, ref useSiteInfo);
1566+
diagnostics.Add(syntax, useSiteInfo);
1567+
1568+
// Unconditionally convert here, to match what we set the ConvertedExpression to in the main BoundForEachStatement node.
1569+
Debug.Assert(!collectionConversion.IsUserDefined);
1570+
collectionExpr = new BoundConversion(
1571+
collectionExpr.Syntax,
1572+
collectionExpr,
1573+
collectionConversion,
1574+
@checked: CheckOverflowAtRuntime,
1575+
explicitCastInCode: false,
1576+
conversionGroupOpt: null,
1577+
ConstantValue.NotAvailable,
1578+
result.Parameters[0].Type);
1579+
1580+
info = BindDefaultArguments(
1581+
result,
1582+
collectionExpr,
1583+
expanded: expanded,
1584+
collectionExpr.Syntax,
1585+
diagnostics);
1586+
}
1587+
else
1588+
{
1589+
info = BindDefaultArguments(
1590+
result,
1591+
extensionReceiverOpt: null,
1592+
expanded: expanded,
1593+
collectionExpr.Syntax,
1594+
diagnostics);
1595+
}
15491596

1550-
var info = BindDefaultArguments(
1551-
result,
1552-
collectionExpr,
1553-
expanded: overloadResolutionResult.ValidResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm,
1554-
collectionExpr.Syntax,
1555-
diagnostics);
15561597
methodGroupResolutionResult.Free();
15571598
analyzedArguments.Free();
15581599
return info;

src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ static SafeContext getDeclarationValEscape(BoundTypeExpression typeExpression, S
604604

605605
static ParameterSymbol? tryGetThisParameter(MethodSymbol method)
606606
{
607-
if (method.IsExtensionMethod)
607+
if (method.IsExtensionMethod) // PROTOTYPE: Test this code path with new extensions
608608
{
609609
return method.Parameters is [{ } firstParameter, ..] ? firstParameter : null;
610610
}

src/Compilers/CSharp/Portable/Binder/WithUsingNamespacesAndTypesBinder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ private static bool IsValidLookupCandidateInUsings(Symbol symbol)
195195

196196
// lookup via "using static" ignores extension methods and non-static methods
197197
case SymbolKind.Method:
198-
if (!symbol.IsStatic || ((MethodSymbol)symbol).IsExtensionMethod)
198+
if (!symbol.IsStatic || ((MethodSymbol)symbol).IsExtensionMethod) // PROTOTYPE: Test this code path with new extensions
199199
{
200200
return false;
201201
}

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,7 +1642,7 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE
16421642
static bool ignoreReceiver(MethodSymbol method)
16431643
{
16441644
// static methods that aren't extensions get an implicit `this` receiver that should be ignored
1645-
return method.IsStatic && !method.IsExtensionMethod;
1645+
return method.IsStatic && !method.IsExtensionMethod; // PROTOTYPE: Test this code path with new extensions
16461646
}
16471647
}
16481648

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ private void VerifyExpression(BoundExpression expression, bool overrideSkippedEx
231231
private void VisitForEachEnumeratorInfo(ForEachEnumeratorInfo enumeratorInfo)
232232
{
233233
Visit(enumeratorInfo.DisposeAwaitableInfo);
234-
if (enumeratorInfo.GetEnumeratorInfo.Method.IsExtensionMethod)
234+
if (enumeratorInfo.GetEnumeratorInfo.Method.IsExtensionMethod) // PROTOTYPE: Test this code path with new extensions
235235
{
236236
foreach (var arg in enumeratorInfo.GetEnumeratorInfo.Arguments)
237237
{

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11042,7 +11042,7 @@ private void VisitForEachExpression(
1104211042

1104311043
MethodSymbol? reinferredGetEnumeratorMethod = null;
1104411044

11045-
if (enumeratorInfoOpt?.GetEnumeratorInfo is { Method: { IsExtensionMethod: true, Parameters: var parameters } } enumeratorMethodInfo)
11045+
if (enumeratorInfoOpt?.GetEnumeratorInfo is { Method: { IsExtensionMethod: true, Parameters: var parameters } } enumeratorMethodInfo) // PROTOTYPE: Test this code path with new extensions
1104611046
{
1104711047
// this is case 7
1104811048
// We do not need to do this same analysis for non-extension methods because they do not have generic parameters that
@@ -11109,7 +11109,7 @@ private void VisitForEachExpression(
1110911109
useLegacyWarnings: false,
1111011110
AssignmentKind.Assignment);
1111111111

11112-
bool reportedDiagnostic = enumeratorInfoOpt?.GetEnumeratorInfo.Method is { IsExtensionMethod: true }
11112+
bool reportedDiagnostic = enumeratorInfoOpt?.GetEnumeratorInfo.Method is { IsExtensionMethod: true } // PROTOTYPE: Test this code path with new extensions
1111311113
? false
1111411114
: CheckPossibleNullReceiver(expr);
1111511115

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.PatternLocalRewriter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ void addArg(RefKind refKind, BoundExpression expression)
169169

170170
Debug.Assert(method.Name == WellKnownMemberNames.DeconstructMethodName);
171171
int extensionExtra;
172-
if (method.IsStatic)
172+
if (method.IsStatic) // PROTOTYPE: Test this code path with new extensions
173173
{
174174
Debug.Assert(method.IsExtensionMethod);
175175
receiver = _factory.Type(method.ContainingType);

0 commit comments

Comments
 (0)