Petr Pokorny

I did not. Git blame only lead to refactoring/cleanup commits and I was too lazy to dig deeper or set up a bisect script. The only side-effect of this change...

Wait, so are there wrong thread static in the `async` / `task` computation that we invoke from `node`, or do they also get corrupted when we're back in `node`? If...

Hmm, so basically we shouldn't have anything that is not `node`-wrapped touch the thread statics. Then it would be ok? Anyway this seems like another good argument to switch to...

Hmm, after investigating this for a bit, it seems to be on purpose...? The diagnostics come from here: https://github.com/dotnet/fsharp/blob/0f520bce120bb2561f67e467646f4abbdb89d60c/src/Compiler/Checking/CheckDeclarations.fs#L5723-L5729 Explicitly having `conditionallySuppressErrorReporting (checkForErrors())` where `checkForErrors` is _A function to help...

Yeah that would make sense. If I remove playing back background diagnostics, only this test fails: https://github.com/dotnet/fsharp/blob/44af7c8141b70e33685fcf58d44145a9d93eb7d7/tests/service/MultiProjectAnalysisTests.fs#L950-L972 https://github.com/dotnet/fsharp/pull/16719 ~Interestingly Transparent Compiler doesn't have this problem.~ Never mind, it does if...

We should probably add a warning when defining a member with the same name as a DU case. We might also want to fix the current error message.

I'm a big fan of getting this in. I'll try to think of some more tests. I'd like to come up with some fuzzing test that would reveal the NRE...

Yeah that might be a good idea. We can also just put `failwith` there.

Good catch. This needs to be fixed probably in both places: - Don't try to replay diagnostics to a Null Logger (though it would be nice to send this to...

> There may be a difference in behavior between the old builder and Transparent Compiler. Possibly there are places where the old one sets a global logger and doesn't bother...