Bind lambda in object initializer despite errors #75695
Conversation
dotnet-issue-labeler
bot
added
the
untriaged
jcouv
force-pushed
the
lambda-initializer
branch
from
6b79fe7 to
e6796a3
Compare
This reverts commit b5969c2.
|
@dotnet/roslyn-compiler for review. Thanks |
| { | ||
| return true; | ||
| } | ||
|
|
||
| // The syntax attached to placeholders should not limit usage of those placeholders | ||
| if (this is not BoundValuePlaceholderBase) |
There was a problem hiding this comment.
While I do not object to making this change, it feels like this is not a fix for the problem the PR is claiming to address. In order for SemanticModel to work properly, every syntax node that is normally bindable should be bound, regardless of errors, That means that the value returned by this helper shouldn't matter. The lambda should be bound even if this helper returns true. #Closed
There was a problem hiding this comment.
We might suppress additional diagnostics based on what this helper returns, but we should not be suppressing the binding.
There was a problem hiding this comment.
I can adjust the PR title if that's what you're suggesting. Like in another lambda-related PR last week, the question is whether we do BindToTypeForErrorRecovery (which results in poor lambda binding) or GenerateConversionForAssignment (which binds a conversion so results in better lambda information).
Maybe "Adjust lambda binding in object initializer despite errors"?
|
Done with review pass (commit 4) |
| @@ -4180,10 +4180,10 @@ public static void Main() | |||
|
|
|||
| var expectedDiagnostics = new[] | |||
| { | |||
| // file.cs(6,28): error CS8386: Invalid object creation | |||
| // (6,28): error CS8386: Invalid object creation | |||
| ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0') | ||
|
|
||
| Right: | ||
| IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Void, IsImplicit) (Syntax: '0') |
There was a problem hiding this comment.
It looks like we call GenerateConversionForAssignment with void in other scenarios too (VoidCall_ImplicitVoidConversion_DiscardAssignment which is a conditional operator, and CS0670ERR_FieldCantHaveVoidType01 which is a field initializer).
I'm leaning to leave as-is. But I'd also be okay to change and file an issue to align/tighten things up.
| @@ -4082,25 +4082,22 @@ void M() | |||
| Statements (0) | |||
| Next (Regular) Block[B1] | |||
| Entering: {R1} | |||
|
|
|||
| .locals {R1} | |||
There was a problem hiding this comment.
It doesn't look like there are meaningful changes in this file. Consider reverting #Closed
| Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(11, 34), | ||
| // (9,13): error CS1688: Cannot convert anonymous method block without a parameter list to delegate type 'OutParam' because it has one or more out parameters | ||
| // o = delegate // CS1688 | ||
| Diagnostic(ErrorCode.ERR_CantConvAnonMethNoParams, "delegate").WithArguments("OutParam").WithLocation(9, 13)); |
There was a problem hiding this comment.
Yes, we have a couple other tests that cover this error
|
Done with review pass (commit 6) |
|
@dotnet/roslyn-compiler for second review. Thanks |
Fixes #72571
The issue is that we get to
BindAssignmentbut the LHS (produced inBindObjectInitializerMemberCommon) has thehasErrorflag set because the implicit receiver (a placeholder with the whole object creation as its syntax) has an error.See
hasErrors = boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors;inBindObjectInitializerMemberCommon.Note: I also tried the approach of attaching a narrower syntax node to the placeholder (commit 2), but I thought it's better to limit the impact of attached syntax on placeholders more generally.