fix metadata failure due to double integration of signature

[dsyme]

Tentative fix for #14179

[dsyme]

I've verified that I can now use functionality from FSharp.Compiler.Service in FSharp.Editor:

[dsyme]

@T-Gro It was a horrible bug several layers deep. I'd like us to consider why this slipped through

• What debug-mode checking we can do on integrity of metadata as we write it, rather than as we read it. For example we could just try to read back in the metadata immediately in debug mode.

• When output metadata lacks integrity (e.g. dangling cross-references) is there any way we can more easily track down the cause of this?

• What testing would have found this earlier. e.g. should we be taking some snapshots of some larger projects (the whole current compiler or something like that) and adding them to the in-memory-cross-project reference tests

• Whether we need so much laziness in the entity_modul_typ field. One of the reasons it was hard to find where things were happening is that it was obscured by this laziness. This is present because it represents the on-demand read/conversion of .NET metadata. But for all the other productions of ModuleOrNamespaceType are strict. The operations like CombineModuleOrNamespaceType that walk and combine these things should really preserve the strictness when all inputs are strict.

[dsyme]

The place where rootSig was already being added to the tcState is

fsharp/src/Compiler/Driver/ParseAndCheckInputs.fs

Lines 1265 to 1268 in 790e3c6

	let ccuSigForFile, tcState =
	AddCheckResultsToTcState
	(tcGlobals, amap, hadSig, prefixPathOpt, tcSink, tcState.tcsTcImplEnv, qualNameOfFile, rootSig)
	tcState

We shouldn't have been adding it again in AddDummyCheckResultsToTcState, which now becomes simple enough that it can be removed altogether.

[TIHan]

LGTM

[vzarytovskii]

/backport to release/dev17.4

[github-actions]

Started backporting to release/dev17.4: https://github.com/dotnet/fsharp/actions/runs/3343596286

[T-Gro]

@T-Gro It was a horrible bug several layers deep. I'd like us to consider why this slipped through

• What debug-mode checking we can do on integrity of metadata as we write it, rather than as we read it. For example we could just try to read back in the metadata immediately in debug mode.
• When output metadata lacks integrity (e.g. dangling cross-references) is there any way we can more easily track down the cause of this?
• What testing would have found this earlier. e.g. should we be taking some snapshots of some larger projects (the whole current compiler or something like that) and adding them to the in-memory-cross-project reference tests
• Whether we need so much laziness in the entity_modul_typ field. One of the reasons it was hard to find where things were happening is that it was obscured by this laziness. This is present because it represents the on-demand read/conversion of .NET metadata. But for all the other productions of ModuleOrNamespaceType are strict. The operations like CombineModuleOrNamespaceType that walk and combine these things should really preserve the strictness when all inputs are strict.

It was at a bad intersection for our testing:

• Only failed in tooling scenario, not in a classical build
• Needed a .fsi file to make the issue visible

Turns out that the test coverage for the intersection of these two properties is small to non-existent.

I think trying to pickle and unpickle our own projects would be a huge improvement already.
Also, surprisingly, the TypeTreePickle's "check" method was launched and did produce thousands of warnings, they just weren't visible anywhere in the tooling scenario.