-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix parsing of for-loops #76476
Fix parsing of for-loops #76476
Conversation
@dotnet/roslyn-compiler @RikkiGibson ptal |
@@ -9181,23 +9181,27 @@ private ForStatementSyntax ParseForStatement(SyntaxList<AttributeListSyntax> att | |||
var (variableDeclaration, initializers) = eatVariableDeclarationOrInitializers(); | |||
|
|||
// Pulled out as we need to track this when parsing incrementors to place skipped tokens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this comment was specific to secondSemicolonToken
before, is it still relevant now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes/ will move.
src/Compilers/CSharp/Test/Syntax/IncrementalParsing/IncrementalParsingTests.cs
Show resolved
Hide resolved
@RikkiGibson this is ready for another pass. |
Fixes #76439
Introduced with #75632. So only in 17.13p2. So fixing this will prevent the regression from making it out.
The core issue here was an incorrect understanding i had with order-of-evaluation and storage of values for args when invoking methods. Specifically, i thought that the following:
Was evaluated effectively as:
But it is not. rather, it is:
In other words, i thought the call to M woudl see the value of 'x' after the call to Z, while it actually evaluates and stores that value prior to the call to Z.
This mattered during parsing as we use that value to store skipped tokens onto. So if we use the value prior to updating, we lose those skipped tokens, leading to an invalid incremental parse.