Skip to content

Commit ed83c92

Browse files
jcouvjcouv
authored
Extensions: improve diagnostics for operators (#80928)
1 parent 9dc51d7 commit ed83c92

34 files changed

+2228
-724
lines changed

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

Lines changed: 353 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,353 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
7+
using Microsoft.CodeAnalysis.CSharp.Symbols;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
using Microsoft.CodeAnalysis.PooledObjects;
10+
11+
namespace Microsoft.CodeAnalysis.CSharp;
12+
13+
internal partial class Binder
14+
{
15+
/// <summary>
16+
/// This type collects different kinds of results from operator scenarios and provides a unified way to report diagnostics.
17+
/// It collects the first non-empty result for extensions and non-extensions separately.
18+
/// This follows a similar logic to ResolveMethodGroupInternal and OverloadResolutionResult.ReportDiagnostics
19+
/// </summary>
20+
private struct OperatorResolutionForReporting
21+
{
22+
private object? _nonExtensionResult;
23+
private object? _extensionResult;
24+
25+
[Conditional("DEBUG")]
26+
private readonly void AssertInvariant()
27+
{
28+
Debug.Assert(_nonExtensionResult is null or OverloadResolutionResult<MethodSymbol> or BinaryOperatorOverloadResolutionResult or UnaryOperatorOverloadResolutionResult);
29+
Debug.Assert(_extensionResult is null or OverloadResolutionResult<MethodSymbol> or BinaryOperatorOverloadResolutionResult or UnaryOperatorOverloadResolutionResult);
30+
}
31+
32+
/// <returns>Returns true if the result was set and <see cref="OperatorResolutionForReporting"/> took ownership of the result.</returns>
33+
private bool SaveResult(object result, ref object? savedResult)
34+
{
35+
if (savedResult is null)
36+
{
37+
savedResult = result;
38+
AssertInvariant();
39+
return true;
40+
}
41+
42+
return false;
43+
}
44+
45+
/// <returns>Returns true if the result was set and <see cref="OperatorResolutionForReporting"/> took ownership of the result.</returns>
46+
public bool SaveResult(OverloadResolutionResult<MethodSymbol> result, bool isExtension)
47+
{
48+
if (result.ResultsBuilder.IsEmpty)
49+
{
50+
return false;
51+
}
52+
53+
return SaveResult(result, ref isExtension ? ref _extensionResult : ref _nonExtensionResult);
54+
}
55+
56+
/// <returns>Returns true if the result was set and <see cref="OperatorResolutionForReporting"/> took ownership of the result.</returns>
57+
public bool SaveResult(BinaryOperatorOverloadResolutionResult result, bool isExtension)
58+
{
59+
if (result.Results.IsEmpty)
60+
{
61+
return false;
62+
}
63+
64+
return SaveResult(result, ref isExtension ? ref _extensionResult : ref _nonExtensionResult);
65+
}
66+
67+
/// <returns>Returns true if the result was set and <see cref="OperatorResolutionForReporting"/> took ownership of the result.</returns>
68+
public bool SaveResult(UnaryOperatorOverloadResolutionResult result, bool isExtension)
69+
{
70+
if (result.Results.IsEmpty)
71+
{
72+
return false;
73+
}
74+
75+
return SaveResult(result, ref isExtension ? ref _extensionResult : ref _nonExtensionResult);
76+
}
77+
78+
/// <summary>
79+
/// Follows a very simplified version of OverloadResolutionResult.ReportDiagnostics which can be expanded in the future if needed.
80+
/// </summary>
81+
internal readonly bool TryReportDiagnostics(SyntaxNode node, Binder binder, object leftDisplay, object? rightDisplay, BindingDiagnosticBag diagnostics)
82+
{
83+
object? resultToUse = pickResultToUse(_nonExtensionResult, _extensionResult);
84+
if (resultToUse is null)
85+
{
86+
return false;
87+
}
88+
89+
var results = ArrayBuilder<(MethodSymbol?, OperatorAnalysisResultKind)>.GetInstance();
90+
populateResults(results, resultToUse);
91+
92+
bool reported = tryReportDiagnostics(node, binder, results, leftDisplay, rightDisplay, diagnostics);
93+
results.Free();
94+
95+
return reported;
96+
97+
static bool tryReportDiagnostics(
98+
SyntaxNode node,
99+
Binder binder,
100+
ArrayBuilder<(MethodSymbol? member, OperatorAnalysisResultKind resultKind)> results,
101+
object leftDisplay,
102+
object? rightDisplay,
103+
BindingDiagnosticBag diagnostics)
104+
{
105+
assertNone(results, OperatorAnalysisResultKind.Undefined);
106+
107+
if (hadAmbiguousBestMethods(results, node, binder, diagnostics))
108+
{
109+
return true;
110+
}
111+
112+
if (results.Any(m => m.resultKind == OperatorAnalysisResultKind.Applicable))
113+
{
114+
return false;
115+
}
116+
117+
assertNone(results, OperatorAnalysisResultKind.Applicable);
118+
119+
if (results.Any(m => m.resultKind == OperatorAnalysisResultKind.Worse))
120+
{
121+
return false;
122+
}
123+
124+
assertNone(results, OperatorAnalysisResultKind.Worse);
125+
126+
Debug.Assert(results.All(r => r.resultKind == OperatorAnalysisResultKind.Inapplicable));
127+
128+
// There is much room to improve diagnostics on inapplicable candidates, but for now we just report the candidate if there is a single one.
129+
if (results is [{ member: { } inapplicableMember }])
130+
{
131+
var toReport = nodeToReport(node);
132+
if (rightDisplay is null)
133+
{
134+
// error: Operator cannot be applied to operand of type '{0}'. The closest inapplicable candidate is '{1}'
135+
Error(diagnostics, ErrorCode.ERR_SingleInapplicableUnaryOperator, toReport, leftDisplay, inapplicableMember);
136+
}
137+
else
138+
{
139+
// error: Operator cannot be applied to operands of type '{0}' and '{1}'. The closest inapplicable candidate is '{2}'
140+
Error(diagnostics, ErrorCode.ERR_SingleInapplicableBinaryOperator, toReport, leftDisplay, rightDisplay, inapplicableMember);
141+
}
142+
143+
return true;
144+
}
145+
146+
return false;
147+
}
148+
149+
static object? pickResultToUse(object? nonExtensionResult, object? extensionResult)
150+
{
151+
if (nonExtensionResult is null)
152+
{
153+
return extensionResult;
154+
}
155+
156+
if (extensionResult is null)
157+
{
158+
return nonExtensionResult;
159+
}
160+
161+
bool useNonExtension = getBestKind(nonExtensionResult) >= getBestKind(extensionResult);
162+
return useNonExtension ? nonExtensionResult : extensionResult;
163+
}
164+
165+
static OperatorAnalysisResultKind getBestKind(object result)
166+
{
167+
OperatorAnalysisResultKind bestKind = OperatorAnalysisResultKind.Undefined;
168+
169+
switch (result)
170+
{
171+
case OverloadResolutionResult<MethodSymbol> r1:
172+
foreach (var res in r1.ResultsBuilder)
173+
{
174+
var kind = mapKind(res.Result.Kind);
175+
if (kind > bestKind)
176+
{
177+
bestKind = kind;
178+
}
179+
}
180+
break;
181+
182+
case BinaryOperatorOverloadResolutionResult r2:
183+
foreach (var res in r2.Results)
184+
{
185+
if (res.Signature.Method is null)
186+
{
187+
// Skip built-in operators
188+
continue;
189+
}
190+
191+
if (res.Kind > bestKind)
192+
{
193+
bestKind = res.Kind;
194+
}
195+
}
196+
break;
197+
198+
case UnaryOperatorOverloadResolutionResult r3:
199+
foreach (var res in r3.Results)
200+
{
201+
if (res.Signature.Method is null)
202+
{
203+
// Skip built-in operators
204+
continue;
205+
}
206+
207+
if (res.Kind > bestKind)
208+
{
209+
bestKind = res.Kind;
210+
}
211+
}
212+
break;
213+
214+
default:
215+
throw ExceptionUtilities.UnexpectedValue(result);
216+
}
217+
218+
return bestKind;
219+
}
220+
221+
static bool hadAmbiguousBestMethods(ArrayBuilder<(MethodSymbol?, OperatorAnalysisResultKind)> results, SyntaxNode node, Binder binder, BindingDiagnosticBag diagnostics)
222+
{
223+
if (!tryGetTwoBest(results, out var first, out var second))
224+
{
225+
return false;
226+
}
227+
228+
Error(diagnostics, ErrorCode.ERR_AmbigOperator, nodeToReport(node), first, second);
229+
return true;
230+
}
231+
232+
static SyntaxNodeOrToken nodeToReport(SyntaxNode node)
233+
{
234+
return node switch
235+
{
236+
AssignmentExpressionSyntax assignment => assignment.OperatorToken,
237+
BinaryExpressionSyntax binary => binary.OperatorToken,
238+
PrefixUnaryExpressionSyntax prefix => prefix.OperatorToken,
239+
PostfixUnaryExpressionSyntax postfix => postfix.OperatorToken,
240+
_ => node
241+
};
242+
}
243+
244+
[Conditional("DEBUG")]
245+
static void assertNone(ArrayBuilder<(MethodSymbol? member, OperatorAnalysisResultKind resultKind)> results, OperatorAnalysisResultKind kind)
246+
{
247+
Debug.Assert(results.All(r => r.resultKind != kind));
248+
}
249+
250+
static bool tryGetTwoBest(ArrayBuilder<(MethodSymbol?, OperatorAnalysisResultKind)> results, [NotNullWhen(true)] out MethodSymbol? first, [NotNullWhen(true)] out MethodSymbol? second)
251+
{
252+
first = null;
253+
second = null;
254+
bool foundFirst = false;
255+
256+
foreach (var (member, resultKind) in results)
257+
{
258+
if (member is null)
259+
{
260+
continue;
261+
}
262+
263+
if (resultKind == OperatorAnalysisResultKind.Applicable)
264+
{
265+
if (!foundFirst)
266+
{
267+
first = member;
268+
foundFirst = true;
269+
}
270+
else
271+
{
272+
Debug.Assert(first is not null);
273+
second = member;
274+
return true;
275+
}
276+
}
277+
}
278+
279+
return false;
280+
}
281+
282+
static void populateResults(ArrayBuilder<(MethodSymbol?, OperatorAnalysisResultKind)> results, object? result)
283+
{
284+
switch (result)
285+
{
286+
case OverloadResolutionResult<MethodSymbol> result1:
287+
foreach (var res in result1.ResultsBuilder)
288+
{
289+
OperatorAnalysisResultKind kind = mapKind(res.Result.Kind);
290+
291+
results.Add((res.Member, kind));
292+
}
293+
break;
294+
295+
case BinaryOperatorOverloadResolutionResult result2:
296+
foreach (var res in result2.Results)
297+
{
298+
results.Add((res.Signature.Method, res.Kind));
299+
}
300+
break;
301+
302+
case UnaryOperatorOverloadResolutionResult result3:
303+
foreach (var res in result3.Results)
304+
{
305+
results.Add((res.Signature.Method, res.Kind));
306+
}
307+
break;
308+
309+
default:
310+
throw ExceptionUtilities.UnexpectedValue(result);
311+
}
312+
}
313+
314+
static OperatorAnalysisResultKind mapKind(MemberResolutionKind kind)
315+
{
316+
return kind switch
317+
{
318+
MemberResolutionKind.ApplicableInExpandedForm => OperatorAnalysisResultKind.Applicable,
319+
MemberResolutionKind.ApplicableInNormalForm => OperatorAnalysisResultKind.Applicable,
320+
MemberResolutionKind.Worse => OperatorAnalysisResultKind.Worse,
321+
MemberResolutionKind.Worst => OperatorAnalysisResultKind.Worse,
322+
_ => OperatorAnalysisResultKind.Inapplicable,
323+
};
324+
}
325+
}
326+
327+
internal void Free()
328+
{
329+
free(ref _nonExtensionResult);
330+
free(ref _extensionResult);
331+
332+
static void free(ref object? result)
333+
{
334+
switch (result)
335+
{
336+
case null:
337+
return;
338+
case OverloadResolutionResult<MethodSymbol> result1:
339+
result1.Free();
340+
break;
341+
case BinaryOperatorOverloadResolutionResult result2:
342+
result2.Free();
343+
break;
344+
case UnaryOperatorOverloadResolutionResult result3:
345+
result3.Free();
346+
break;
347+
}
348+
349+
result = null;
350+
}
351+
}
352+
}
353+
}

0 commit comments

Comments
 (0)