Collection expressions: report specific conversion errors from overload resolution #73307
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
|
@dotnet/roslyn-compiler, please review. |
| @@ -1231,6 +1231,10 @@ private static bool HadLambdaConversionError(BindingDiagnosticBag diagnostics, A | |||
| { | |||
| // a diagnostic has been reported by ReportDelegateOrFunctionPointerMethodGroupDiagnostics | |||
| } | |||
| else if (argument.Kind == BoundKind.UnconvertedCollectionExpression) | |||
| { | |||
| binder.GenerateImplicitConversionErrorForCollectionExpression((BoundUnconvertedCollectionExpression)argument, parameterType, diagnostics); | |||
| { | ||
| string source = """ | ||
| ERROR[] values = [1]; | ||
| values = [2]; |
There was a problem hiding this comment.
That's interesting, and seems reasonable I guess, as an "avoid cascading" behavior? Is this similar to how values = new[] { 2 } would behave?
There was a problem hiding this comment.
Yes, the behavior is similar for values = new[] { 2 } which doesn't result in an error (since it is assumed an error was reported on the declaration of ERROR[] values;.
However, the error that is suppressed in values = new[] { 2 } is converting int[] to ERROR[], whereas with values = [2] the error that is suppressed is converting int to ERROR.
Also, the error reporting for assignment did not change in this PR - I added this test to show the similar diagnostics to the previous test InAttribute_BadArrayType() which was affected by this PR. In that test, a call to [X(new[] { 2 })] will result in a conversion error for the argument.
I'm on the fence about whether we should report an error at the for [X([1])] in that test.
There was a problem hiding this comment.
However, the error that is suppressed in
values = new[] { 2 }is convertingint[]toERROR[], whereas withvalues = [2]the error that is suppressed is convertinginttoERROR.
Another example is values = default which does not result in any additional error.
There was a problem hiding this comment.
that all sounds good, merge at will :)
|
/azp run roslyn-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #73297