Fix issue with frozen-partial snapshots having incomplete references. #73205
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
commented
Apr 24, 2024
| { | ||
| // if we have a compilation and its the correct language, use a simple compilation reference in any | ||
| // state it happens to be in right now | ||
| if (ReadState() is CompilationTrackerState compilationState) |
There was a problem hiding this comment.
this was the crux of the bug. the CompilationTrackerState we fetched here would have the compilations containing the source-documents, but not the referenced metadata (either PE references, or P2P references).
I considered updating this logic to add that in. But that would then effectively be doing what FinalizeCompilation already does. So i uniformly handled it at that layer.
CyrusNajmabadi
commented
Apr 24, 2024
| @@ -1046,6 +1028,9 @@ public Task<bool> HasSuccessfullyLoadedAsync(ProjectState project, CancellationT | |||
| return compilation.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); | |||
| } | |||
There was a problem hiding this comment.
this now always succeed when it's from teh same language. the call to tracker.GetCompilationAsync above will ensure that you get back a compilation with all the references properly added (modulo respecting the 'generate skeletons' flag).
|
@jasonmalinowski For review when you get back. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2035575
Core issue is that when producing a frozen partial snapshot, we had uniform logic for producing P2P references, regardless of whether the P2P reference was to the same language or a different language.
For frozen partial, we don't want to produce the reference to a different language, as that would involve generating skeleton assemblies. This is enormously expensive as it is effectively running the compiler and emitting the metadata to an in-memory object we then read in as a reference.
However, for the same language, making a normal P2P reference is fine (and cheap). In that case, we can just point directly at the compilation object directly, as the compilers support direct (same lang) p2p refs.
In the case of frozen-partial, because we already forked all compilation-trackers to not run generators or produce skeletons, asking for the compilation for the reference will not incur those costs (transitively). So frozen partial will still be fast for the mainline case, while avoiding the massive expensvie of SGs and Skeletons at all levels.
--
This was breaking a cross project winforms case when the project config was changed. Namely, after changing the project config, we would try to determine which were the designable types in the solution. Finding a type X in P1, we would then try to walk its inheritance chain. If that type X inherited from a type Y in P2, and we hadn't produced a viable P2P ref for that, we'd end up with error types, breaking the designable type detection rules.
In the case here, while we had a P2P ref, because we simply snapped that P2P ref at the point it was at, we never did things like properly add all the metadata-references that project had, leading to a referenced compilation with sources in it, but no metadata.