So it's a bit worrisome for me that we're throwing away the SemanticModel here. If the full Compilation was available and we didn't actually do the freeze operation, the fact we've thrown away the SemanticModel could mean that theoretically the SemanticModel and Compilation could GC, and then the request for the SemanticModel has to do extra work. It won't run generators. Put another way, throwing away the SemanticModel here is potentially undoing the optimization that GetPartialSemanticModelAsync is trying to do in the first place. This would require a pretty badly timed GC though.

I'm suspicious of GetPartialSemanticModelAsync in general so maybe we should just delete it if that seems easier, but I'm happy for that to be done in another PR.

That's a good point, I will hold on to the reference to returned model until completion providers finished their work

and I will leave cleaning up GetPartialSemanticModelAsync to a follow up PR

You don't still want a partial semantic model in this case? Note that a "partial" model is different than the "speculative" model that the comment mentions; the speculative guarantees the tree you have will be up to date for the file, which won't run into what I think that comment is worried about.

If you really do still need the full model, then revise the comment since the comment doesn't match the code anymore.

The document passed in is frozen, so I'd think GetRequiredSemanticModelAsync would return partial model, right? Providers don't know if they are getting full model or partial model, so regular here is in contrast to speculative, and I believe the comment is still accurate in that sense

Same comment as C# side.
#55745 (comment)

You need to GC.KeepAlive this.

Oh, interesting... Let me try someting.

I'd like to see this:

Should be easy with this helper type:

internal readonly struct KeepAlive : IDisposable
{
    private readonly object? _instance;

    public KeepAlive(object? instance)
        => _instance = instance;

    public void Dispose()
        => GC.KeepAlive(_instance);
}

        //     References the specified object, which makes it ineligible for garbage collection
        //     from the start of the current routine to the point where this method is called.

Based on the doc, the helper type only guarantee _instance to be GC ineligible after Dispose is called at the end of current method, right? It can still get GC'd before then, unless Dispose is inlined

The call to Dispose will occur immediately before returning, which is where we want the call to KeepAlive placed.

from the start of the current routine

Wouldn't that make current routine be Dispose instead in this case?