Fix namespace incremental parsing bug#37770
Conversation
|
:( Sorry about that. |
|
So previous to this, was it the case that we basically wouldn't reuse the namespace decl because it had errors on it. But because there were no longer errors, we thought we could use it now? Definitely a subtle thing i hadn't considered. |
src/Compilers/CSharp/Test/Syntax/IncrementalParsing/IncrementalParsingTests.cs
Outdated
Show resolved
Hide resolved
|
@jasonmalinowski indicated that this should be taken in 16.2 servicing |
jasonmalinowski
left a comment
There was a problem hiding this comment.
This needs to target 16.2. Feel free to dismiss this once that's been done.
There was a previous parsing change (dotnet#32999) which modified namespace parsing to allow modifiers and attributes on namespaces, to improve error recovery. This PR contained a bug because it didn't move the incremental parsing check to before parsing attributes and modifiers, which should now be included in incremental parsing to prevent changes from being dropped Fixes dotnet#37665, dotnet#37664, dotnet#37663
63f5349 to
4b6ca77
Compare
|
@CyrusNajmabadi No worries, now I got to fix my first incremental parsing bug 😄 I think you're right, this was probably working before because the modifiers and attributes were errors. |
|
Actually, the modifiers and attributes were previously stored as skipped syntax, so they weren't part of the node at all. So the namespace node reuse was fine. |
|
Ah, but that still would have dropped tokens. No, it's probably the errors. |
|
@agocke I noticed the issues marked as "will be fixed by this PR" are still open. Do you know what's going on there? |
|
Probably just hasn't made it to master. I think that's when bugs get autoclosed. |
There was a previous parsing change (#32999) which modified namespace
parsing to allow modifiers and attributes on namespaces, to improve
error recovery.
This PR contained a bug because it didn't move the incremental parsing
check to before parsing attributes and modifiers, which should now
be included in incremental parsing to prevent changes from being dropped
Fixes #37665, #37664, #37663