Skip to content

Commit ddc83e5

Browse files
AlekseyTsAlekseyTs
authored
Address follow-up comments for extension operators (#79249)
Closes #79136.
1 parent 2b7ef41 commit ddc83e5

File tree

2 files changed

+666
-10
lines changed

2 files changed

+666
-10
lines changed

‎src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,15 @@ private BoundExpression BindCompoundAssignment(AssignmentExpressionSyntax node,
151151
OverloadResolution.GetStaticUserDefinedBinaryOperatorMethodNames(kind, checkOverflowAtRuntime, out string staticOperatorName1, out string? staticOperatorName2Opt);
152152
BinaryOperatorAnalysisResult best = BinaryOperatorNonExtensionOverloadResolution(kind, isChecked: checkOverflowAtRuntime, staticOperatorName1, staticOperatorName2Opt, left, right, node, diagnostics, out resultKind, out originalUserDefinedOperators);
153153

154+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
155+
Debug.Assert(best.HasValue == (resultKind is LookupResultKind.Viable));
156+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
157+
154158
if (!best.HasValue && resultKind != LookupResultKind.Ambiguous)
155159
{
160+
Debug.Assert(resultKind is LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
161+
Debug.Assert(originalUserDefinedOperators.IsEmpty);
162+
156163
LookupResultKind staticExtensionResultKind;
157164
ImmutableArray<MethodSymbol> staticExtensionOriginalUserDefinedOperators;
158165
BinaryOperatorAnalysisResult? staticExtensionBest;
@@ -165,12 +172,11 @@ private BoundExpression BindCompoundAssignment(AssignmentExpressionSyntax node,
165172

166173
if (instanceExtensionResult is not null)
167174
{
168-
// https://github.com/dotnet/roslyn/issues/79136: If this result is bad, we might want to stick with bad non-extension result as we do for static extensions below.
175+
Debug.Assert(instanceExtensionResult.ResultKind is LookupResultKind.Viable || !instanceExtensionResult.OriginalUserDefinedOperatorsOpt.IsDefaultOrEmpty);
169176
return instanceExtensionResult;
170177
}
171178

172-
// https://github.com/dotnet/roslyn/issues/79136: Add test coverage for the choice between two bad results
173-
if (staticExtensionBest.HasValue && (staticExtensionBest.GetValueOrDefault().HasValue || (originalUserDefinedOperators.IsEmpty && !staticExtensionOriginalUserDefinedOperators.IsEmpty)))
179+
if (staticExtensionBest.HasValue)
174180
{
175181
best = staticExtensionBest.GetValueOrDefault();
176182
resultKind = staticExtensionResultKind;
@@ -1966,13 +1972,20 @@ private BinaryOperatorAnalysisResult BinaryOperatorOverloadResolution(
19661972
{
19671973
BinaryOperatorAnalysisResult possiblyBest = BinaryOperatorNonExtensionOverloadResolution(kind, isChecked, name1, name2Opt, left, right, node, diagnostics, out resultKind, out originalUserDefinedOperators);
19681974

1975+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
1976+
Debug.Assert(possiblyBest.HasValue == (resultKind is LookupResultKind.Viable));
1977+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
1978+
19691979
if (!possiblyBest.HasValue && resultKind != LookupResultKind.Ambiguous)
19701980
{
1981+
Debug.Assert(resultKind is LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
1982+
Debug.Assert(originalUserDefinedOperators.IsEmpty);
1983+
19711984
LookupResultKind extensionResultKind;
19721985
ImmutableArray<MethodSymbol> extensionOriginalUserDefinedOperators;
19731986
BinaryOperatorAnalysisResult? extensionBest = BinaryOperatorExtensionOverloadResolution(kind, isChecked, name1, name2Opt, left, right, node, diagnostics, out extensionResultKind, out extensionOriginalUserDefinedOperators);
19741987

1975-
if (extensionBest.HasValue && (extensionBest.GetValueOrDefault().HasValue || (originalUserDefinedOperators.IsEmpty && !extensionOriginalUserDefinedOperators.IsEmpty)))
1988+
if (extensionBest.HasValue)
19761989
{
19771990
possiblyBest = extensionBest.GetValueOrDefault();
19781991
resultKind = extensionResultKind;
@@ -2051,6 +2064,10 @@ private BinaryOperatorAnalysisResult BinaryOperatorNonExtensionOverloadResolutio
20512064
var possiblyBest = BinaryOperatorAnalyzeOverloadResolutionResult(result, out resultKind, out originalUserDefinedOperators);
20522065
result.Free();
20532066

2067+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
2068+
Debug.Assert(possiblyBest.HasValue == (resultKind is LookupResultKind.Viable));
2069+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
2070+
20542071
return possiblyBest;
20552072
}
20562073

@@ -2142,13 +2159,20 @@ private UnaryOperatorAnalysisResult UnaryOperatorOverloadResolution(
21422159

21432160
var best = UnaryOperatorNonExtensionOverloadResolution(kind, isChecked, name1, name2Opt, operand, node, diagnostics, out resultKind, out originalUserDefinedOperators);
21442161

2162+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
2163+
Debug.Assert(best.HasValue == (resultKind is LookupResultKind.Viable));
2164+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
2165+
21452166
if (!best.HasValue && resultKind != LookupResultKind.Ambiguous)
21462167
{
2168+
Debug.Assert(resultKind is LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
2169+
Debug.Assert(originalUserDefinedOperators.IsEmpty);
2170+
21472171
LookupResultKind extensionResultKind;
21482172
ImmutableArray<MethodSymbol> extensionOriginalUserDefinedOperators;
21492173
UnaryOperatorAnalysisResult? extensionBest = this.UnaryOperatorExtensionOverloadResolution(kind, isChecked, name1, name2Opt, operand, node, diagnostics, out extensionResultKind, out extensionOriginalUserDefinedOperators);
21502174

2151-
if (extensionBest.HasValue && (extensionBest.GetValueOrDefault().HasValue || (originalUserDefinedOperators.IsEmpty && !extensionOriginalUserDefinedOperators.IsEmpty)))
2175+
if (extensionBest.HasValue)
21522176
{
21532177
best = extensionBest.GetValueOrDefault();
21542178
resultKind = extensionResultKind;
@@ -2178,6 +2202,11 @@ private UnaryOperatorAnalysisResult UnaryOperatorNonExtensionOverloadResolution(
21782202
UnaryOperatorAnalysisResult possiblyBest = AnalyzeUnaryOperatorOverloadResolutionResult(result, kind, operand, node, diagnostics, out resultKind, out originalUserDefinedOperators);
21792203

21802204
result.Free();
2205+
2206+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
2207+
Debug.Assert(possiblyBest.HasValue == (resultKind is LookupResultKind.Viable));
2208+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
2209+
21812210
return possiblyBest;
21822211
}
21832212

@@ -3155,8 +3184,15 @@ private BoundExpression BindIncrementOperator(ExpressionSyntax node, ExpressionS
31553184
ImmutableArray<MethodSymbol> originalUserDefinedOperators;
31563185
var best = this.UnaryOperatorNonExtensionOverloadResolution(kind, isChecked, staticOperatorName1, staticOperatorName2Opt, operand, node, diagnostics, out resultKind, out originalUserDefinedOperators);
31573186

3187+
Debug.Assert(resultKind is LookupResultKind.Viable or LookupResultKind.Ambiguous or LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
3188+
Debug.Assert(best.HasValue == (resultKind is LookupResultKind.Viable));
3189+
Debug.Assert(resultKind is not (LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty) || originalUserDefinedOperators.IsEmpty);
3190+
31583191
if (!best.HasValue && resultKind != LookupResultKind.Ambiguous)
31593192
{
3193+
Debug.Assert(resultKind is LookupResultKind.OverloadResolutionFailure or LookupResultKind.Empty);
3194+
Debug.Assert(originalUserDefinedOperators.IsEmpty);
3195+
31603196
// Check for extension operators
31613197
LookupResultKind staticExtensionResultKind;
31623198
ImmutableArray<MethodSymbol> staticExtensionOriginalUserDefinedOperators;
@@ -3170,12 +3206,11 @@ private BoundExpression BindIncrementOperator(ExpressionSyntax node, ExpressionS
31703206

31713207
if (instanceExtensionResult is not null)
31723208
{
3173-
// https://github.com/dotnet/roslyn/issues/79136: If this result is bad, we might want to stick with bad non-extension result as we do for static extensions below.
3209+
Debug.Assert(instanceExtensionResult.ResultKind is LookupResultKind.Viable || !instanceExtensionResult.OriginalUserDefinedOperatorsOpt.IsDefaultOrEmpty);
31743210
return instanceExtensionResult;
31753211
}
31763212

3177-
// https://github.com/dotnet/roslyn/issues/79136: Add test coverage for the choice between two bad results
3178-
if (staticExtensionBest.HasValue && (staticExtensionBest.GetValueOrDefault().HasValue || (originalUserDefinedOperators.IsEmpty && !staticExtensionOriginalUserDefinedOperators.IsEmpty)))
3213+
if (staticExtensionBest.HasValue)
31793214
{
31803215
best = staticExtensionBest.GetValueOrDefault();
31813216
resultKind = staticExtensionResultKind;
@@ -3699,7 +3734,7 @@ static void lookupUserDefinedInstanceExtensionOperatorsInSingleScope(
36993734
!((extensionParameter.Type.IsValueType && extensionParameter.RefKind == RefKind.Ref) ||
37003735
(extensionParameter.Type.IsReferenceType && extensionParameter.RefKind == RefKind.None)))
37013736
{
3702-
continue; // https://github.com/dotnet/roslyn/issues/79136: Test this code path for actually interesting scenarios around RefKind which have an observable effect vs. just an optimization.
3737+
continue;
37033738
}
37043739

37053740
typeOperators ??= ArrayBuilder<MethodSymbol>.GetInstance();
@@ -3711,7 +3746,7 @@ static void lookupUserDefinedInstanceExtensionOperatorsInSingleScope(
37113746
// If we're in error recovery, we might have bad operators. Just ignore them.
37123747
if (op.IsStatic || !IsViableInstanceOperator(op, parameterCount))
37133748
{
3714-
continue; // https://github.com/dotnet/roslyn/issues/79136: Add test coverage for this code path
3749+
continue;
37153750
}
37163751

37173752
methods ??= ArrayBuilder<MethodSymbol>.GetInstance();

0 commit comments

Comments
 (0)