Fix post init trees parsing when parse options change #61934
Conversation
chsienki
force-pushed
the
source-generators/post-init-reparse
branch
from
47bce20 to
ce9a293
Compare
|
@dotnet/roslyn-compiler for review please. |
|
Ping @dotnet/roslyn for review please :) |
|
|
||
| public GeneratorDriver WithUpdatedParseOptions(ParseOptions newOptions) => newOptions is object | ||
| ? FromState(_state.With(parseOptions: newOptions)) | ||
| ? FromState(_state.With(parseOptions: newOptions, parseOptionsChanged: true)) |
There was a problem hiding this comment.
While ParseOptions doesn't implement any equality all of the derived types do implement equality. Yet here we're unconditionally saying it changed. Do these changes happen frequently enough that we should plumb this through the derived types to let them say if the new ParseOptions are different than the current ones?
There was a problem hiding this comment.
I may have to implement equality checking in #62219 to make that approach work there, so if we don't do it here this may not last long. I have new bits to test that PR with so will hopefully know soon enough if it works or needs that change.
There was a problem hiding this comment.
We could, but re-parsing the init trees is cheap, and doesn't happen often enough that it'll be an issue. I think we should not block on this here, and can always improve it at a later date if we need to.
| driver = driver.AddAdditionalTexts(ImmutableArray<AdditionalText>.Empty); | ||
|
|
||
| driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out compilation, out diagnostics); | ||
| diagnostics.Verify(); |
There was a problem hiding this comment.
There should be diagnostics at this stage though because the language version is csharp 1. Believe we should validate these diagonstics show up here, likely by just grabbing the diagnostics off the syntax tree in the compilation.
There was a problem hiding this comment.
These are the generator diagnostics. I'll add a check for the compilation.
|
@chsienki Any update on this one? This is the top NFW for our .NET 6 remote host in dev17.4 |
|
Upping priority. @chsienki can you please prioritize accordingly? Thanks! |
chsienki
force-pushed
the
source-generators/post-init-reparse
branch
from
ce9a293 to
0f3deae
Compare
|
Azure Pipelines successfully started running 2 pipeline(s). |
ghost
added this to the
8 participants
If the parse options change after initialize has been called, we were not re-parsing the post init trees, leading to
System.ArgumentException: Inconsistent language versionsbeing thrown.This tracks when the parse options change, and re-parses the init trees in the next generation pass if they have.