Short term aim: Remove use of reflection to determine target assembly list from host

[dsyme]

We are discussing lingering issues with the TPSDK. One of these include the god-awful use of reflection to access the internals of the host tooling to determine the list of referenced assemblies in the target assembly context. It is a great crime that was committed "for reasons"

Those reasons were fixed in 2016 with https://github.com/Microsoft/visualfsharp/pull/1037

(OK, this hack was added because we were on a time-critical delivery for cross-targeting type providers and there was no way we could push through an update to FSharp.Core and the F# compiler tooling to report this information correctly)

We should remove/deprecate this hack by making an improvement to the F# compiler to report this information to the type provider.

The issue in VF# was

[cartermp]

Separately, there is a memory leak @TIHan identified and can fix, but doing so would break all type providers today due to this reflection code. The memory leak is critically important to fix, as any leaks could very well be the cause of near-undiagnosable "VS just OOMed on me" reports we get. But we haven't done it yet due to an update of the compiler breaking all type providers. Even if we went and updated this and all important TPs, there would be a tail of people not updating their project dependencies, using the newer compiler through tools, and seeing everything as busted.

What do you think is the preferred order of operations here?

[dsyme]

Separately, there is a memory leak @TIHan identified and can fix, but doing so would break all type providers today due to this reflection code. The memory leak is critically important to fix, as any leaks could very well be the cause of near-undiagnosable "VS just OOMed on me" reports we get. But we haven't done it yet due to an update of the compiler breaking all type providers. Even if we went and updated this and all important TPs, there would be a tail of people not updating their project dependencies, using the newer compiler through tools, and seeing everything as busted. What do you think is the preferred order of operations here?

I'm sure there will be some awful way to fix the memory leak without breaking this hack. I'll work with @TIHan on it

But we should absolutely, definitely remove/deprecate the hack so it eventually flushes out of the system.

[dsyme]

I'm sure there will be some awful way to fix the memory leak without breaking this hack. I'll work with @TIHan on it

Specifically, the hack imposes some loose constraints on the systemRuntimeContainsType object. We can make use of the fact that these are somewhat loose to ensure we get the right result here, while also inserting the weak reference. I'll look at @TIHan's PR.

[dsyme]

@TIHan @cartermp OK, this looks promising, #305 passes locally

So we should just be able to remove this hack immediately, as the fix to config.ReferencedAssemblies was shipped in VS2017 and we don't assume anyone is using TPSDK for type providers targeting tooling before that.

Note that "visualfsharp" contains a copy of ProvidedTypes.fs which will also need to be updated (or we publish a nuget package from this repo and pick up the file as source).

[cartermp]

Would removing it break users on VS2017 who update their type provider package?

[dsyme]

Would removing it break users on VS2017 who update their type provider package?

No. It might break users of VS2015

[TIHan]

Now that this is in, we can test the WeakReference of TcImports.

[dsyme]

See also https://github.com/fsharp/FSharp.Data/pull/1258, we'll get that through to nuget

[cartermp]

@dsyme should this be closed?