Adjust scope of declarations within a Labeled and Local Function Statements according to the latest design. #13273
Conversation
…to the latest design. Related to dotnet#12597.
…ing to the latest design. Related to dotnet#12597.
…o the latest design. Related to dotnet#12597.
… the latest design. Related to dotnet#12597.
|
Since no feedback has been provided for this small change, I added two new commits to handle Return/Throw statements. |
|
Looking at this now. In reply to: 241485281 [](ancestors = 241485281) |
| } | ||
| } | ||
| break; | ||
|
|
||
| case SyntaxKind.ExpressionStatement: | ||
| case SyntaxKind.IfStatement: | ||
| case SyntaxKind.YieldReturnStatement: | ||
| case SyntaxKind.ReturnStatement: | ||
| case SyntaxKind.ThrowStatement: | ||
| ExpressionVariableFinder.FindExpressionVariables(this, locals, innerStatement, enclosingBinder.GetBinder(innerStatement) ?? enclosingBinder); |
There was a problem hiding this comment.
enclosingBinder.GetBinder(innerStatement) [](start = 103, length = 41)
Under what conditions would this return a non-null result, and under which conditions a null result? I'm guessing it has to do with whether or not the SyntaxList<StatementSyntax> statements passed in comes from the real tree or not, but some comment around here would be helpful, please.
|
|
| return new BoundBlock(statement.Syntax, locals, localFunctions, | ||
| ImmutableArray.Create(statement)) | ||
| { WasCompilerGenerated = true }; | ||
| } |
There was a problem hiding this comment.
To avoid the problem of documenting and knowing whether to call WrapWithVariablesIfAny or the new method, maybe the new logic could be added to the existing method instead.
The caller would always use WrapWithVariablesIfAny and that existing method would be smart to check if the scope could declare local functions.
There was a problem hiding this comment.
The check to know "if the scope could declare local functions" wouldn't be simple. Besides, the code and APIs in Binder are intentionally structured in such a way so that requests for locals and local functions were intentional and only in right contexts.
|
LGTM Thanks |
Related to #12597.
Plus addressing feedback from previous PR.
@dotnet/roslyn-compiler, @gafter Please review.