From 9251b4c2294ed31b6a51b67986c5437aa537ee41 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 23 Feb 2022 16:06:50 -0800 Subject: [PATCH 1/3] Expose the underlying ICompilationTracker from a GeneratedFileReplacingCompilationTracker This is just a simple conversion of the field to a property. --- ...eneratedFileReplacingCompilationTracker.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs index ff751699e5a1a..85ac5e30afb5f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.GeneratedFileReplacingCompilationTracker.cs @@ -20,7 +20,6 @@ internal partial class SolutionState /// private class GeneratedFileReplacingCompilationTracker : ICompilationTracker { - private readonly ICompilationTracker _underlyingTracker; private readonly SourceGeneratedDocumentState _replacedGeneratedDocumentState; private AsyncLazy? _lazyDependentChecksum; @@ -32,17 +31,18 @@ private class GeneratedFileReplacingCompilationTracker : ICompilationTracker [DisallowNull] private Compilation? _compilationWithReplacement; + public ICompilationTracker UnderlyingTracker { get; } public SkeletonReferenceCache SkeletonReferenceCache { get; } - public ProjectState ProjectState => _underlyingTracker.ProjectState; + public ProjectState ProjectState => UnderlyingTracker.ProjectState; public GeneratedFileReplacingCompilationTracker(ICompilationTracker underlyingTracker, SourceGeneratedDocumentState replacementDocumentState) { - _underlyingTracker = underlyingTracker; + UnderlyingTracker = underlyingTracker; _replacedGeneratedDocumentState = replacementDocumentState; SkeletonReferenceCache = underlyingTracker.SkeletonReferenceCache.Clone(); } - public GeneratorDriver? GeneratorDriver => _underlyingTracker.GeneratorDriver; + public GeneratorDriver? GeneratorDriver => UnderlyingTracker.GeneratorDriver; public bool ContainsAssemblyOrModuleOrDynamic(ISymbol symbol, bool primary) { @@ -79,8 +79,8 @@ public async Task GetCompilationAsync(SolutionState solution, Cance return _compilationWithReplacement; } - var underlyingCompilation = await _underlyingTracker.GetCompilationAsync(solution, cancellationToken).ConfigureAwait(false); - var underlyingSourceGeneratedDocuments = await _underlyingTracker.GetSourceGeneratedDocumentStatesAsync(solution, cancellationToken).ConfigureAwait(false); + var underlyingCompilation = await UnderlyingTracker.GetCompilationAsync(solution, cancellationToken).ConfigureAwait(false); + var underlyingSourceGeneratedDocuments = await UnderlyingTracker.GetSourceGeneratedDocumentStatesAsync(solution, cancellationToken).ConfigureAwait(false); underlyingSourceGeneratedDocuments.TryGetState(_replacedGeneratedDocumentState.Id, out var existingState); @@ -110,10 +110,10 @@ public async Task GetCompilationAsync(SolutionState solution, Cance } public Task GetDependentVersionAsync(SolutionState solution, CancellationToken cancellationToken) - => _underlyingTracker.GetDependentVersionAsync(solution, cancellationToken); + => UnderlyingTracker.GetDependentVersionAsync(solution, cancellationToken); public Task GetDependentSemanticVersionAsync(SolutionState solution, CancellationToken cancellationToken) - => _underlyingTracker.GetDependentSemanticVersionAsync(solution, cancellationToken); + => UnderlyingTracker.GetDependentSemanticVersionAsync(solution, cancellationToken); public Task GetDependentChecksumAsync(SolutionState solution, CancellationToken cancellationToken) { @@ -129,7 +129,7 @@ public Task GetDependentChecksumAsync(SolutionState solution, Cancella private async Task ComputeDependentChecksumAsync(SolutionState solution, CancellationToken cancellationToken) => Checksum.Create( - await _underlyingTracker.GetDependentChecksumAsync(solution, cancellationToken).ConfigureAwait(false), + await UnderlyingTracker.GetDependentChecksumAsync(solution, cancellationToken).ConfigureAwait(false), await _replacedGeneratedDocumentState.GetChecksumAsync(cancellationToken).ConfigureAwait(false)); public CompilationReference? GetPartialMetadataReference(ProjectState fromProject, ProjectReference projectReference) @@ -146,7 +146,7 @@ await _underlyingTracker.GetDependentChecksumAsync(solution, cancellationToken). public async ValueTask> GetSourceGeneratedDocumentStatesAsync(SolutionState solution, CancellationToken cancellationToken) { - var underlyingGeneratedDocumentStates = await _underlyingTracker.GetSourceGeneratedDocumentStatesAsync(solution, cancellationToken).ConfigureAwait(false); + var underlyingGeneratedDocumentStates = await UnderlyingTracker.GetSourceGeneratedDocumentStatesAsync(solution, cancellationToken).ConfigureAwait(false); if (underlyingGeneratedDocumentStates.Contains(_replacedGeneratedDocumentState.Id)) { @@ -166,7 +166,7 @@ public async ValueTask> GetSour public Task HasSuccessfullyLoadedAsync(SolutionState solution, CancellationToken cancellationToken) { - return _underlyingTracker.HasSuccessfullyLoadedAsync(solution, cancellationToken); + return UnderlyingTracker.HasSuccessfullyLoadedAsync(solution, cancellationToken); } public bool TryGetCompilation([NotNullWhen(true)] out Compilation? compilation) @@ -183,7 +183,7 @@ public bool TryGetCompilation([NotNullWhen(true)] out Compilation? compilation) } else { - return _underlyingTracker.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(documentId); + return UnderlyingTracker.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(documentId); } } } From e316c4217a7d9144d410c5d5d8ebef179620332f Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 23 Feb 2022 16:17:13 -0800 Subject: [PATCH 2/3] Don't lose the frozen source generated document state In this particular code path, we were passing null for the frozen source generated document state when we could have passed it along. The original logic here was frozen source generated documents only normally appear when somebody calls ITextSnapshot.GetOpenDocumentInCurrentContextWithChanges, since we need to ensure that there is a generated document with that exact snapshot for the feature to work. We should never see that in a "primary" solution in any way. However, solution sync uses CurrentSolution as a cache for the previously synced complete solution, which meant we'd still end up calling this anyways. Although we could (and probably should) avoid caching these special solutions entirely, this avoids any surprises here and makes things behave like normal, and avoids odd cases where the CompilationTrackers are out of sync with this field. This partially addresses https://github.com/dotnet/roslyn/issues/57082 --- .../Core/Portable/Workspace/Solution/SolutionState.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index e79f594cd88c7..cfa66a9c6a946 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -314,7 +314,7 @@ private SolutionState CreatePrimarySolution( _filePathToDocumentIdsMap, _dependencyGraph, _lazyAnalyzers, - frozenSourceGeneratedDocument: null); + _frozenSourceGeneratedDocumentState); } private BranchId GetBranchId() From 314bfbc95b0b90f6e713ff9b73e813d4a53db615 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 23 Feb 2022 16:20:13 -0800 Subject: [PATCH 3/3] Correctly handle a cached solution having frozen generated documents When we are synchronizing solutions, the previous assumption in this code was incorrect -- we absolutely can end up with a frozen generated document in CurrentSolution, since the remote workspace caches things there. We're going to just run with that and consider that normal, so we'll fix up the code to support it cleanly. Fixes https://github.com/dotnet/roslyn/issues/57082 Fixes https://github.com/dotnet/roslyn/issues/56998 --- .../Services/SolutionServiceTests.cs | 21 ++++++++++++---- .../Portable/Workspace/Solution/Solution.cs | 15 +++++++++++ .../Workspace/Solution/SolutionState.cs | 25 +++++++++++++++++++ .../Host/RemoteWorkspace.SolutionCreator.cs | 8 +++--- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs b/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs index c6854515d50e2..106bc3236765c 100644 --- a/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/SolutionServiceTests.cs @@ -486,18 +486,29 @@ public async Task TestFrozenSourceGeneratedDocument() .AddAnalyzerReference(new AnalyzerFileReference(typeof(Microsoft.CodeAnalysis.TestSourceGenerator.HelloWorldGenerator).Assembly.Location, new TestAnalyzerAssemblyLoader())) .Solution; + // First sync the solution over that has a generator var assetProvider = await GetAssetProviderAsync(workspace, remoteWorkspace, solution); var solutionChecksum = await solution.State.GetChecksumAsync(CancellationToken.None); var synched = await remoteWorkspace.GetSolutionAsync(assetProvider, solutionChecksum, fromPrimaryBranch: true, workspaceVersion: 0, projectId: null, CancellationToken.None); Assert.Equal(solutionChecksum, await synched.State.GetChecksumAsync(CancellationToken.None)); + // Now freeze with some content var documentIdentity = (await solution.Projects.Single().GetSourceGeneratedDocumentsAsync()).First().Identity; - var frozenText = SourceText.From("// Hello, World!"); - solution = solution.WithFrozenSourceGeneratedDocument(documentIdentity, frozenText).Project.Solution; + var frozenText1 = SourceText.From("// Hello, World!"); + var frozenSolution1 = solution.WithFrozenSourceGeneratedDocument(documentIdentity, frozenText1).Project.Solution; - assetProvider = await GetAssetProviderAsync(workspace, remoteWorkspace, solution); - solutionChecksum = await solution.State.GetChecksumAsync(CancellationToken.None); - synched = await remoteWorkspace.GetSolutionAsync(assetProvider, solutionChecksum, fromPrimaryBranch: false, workspaceVersion: 1, projectId: null, CancellationToken.None); + assetProvider = await GetAssetProviderAsync(workspace, remoteWorkspace, frozenSolution1); + solutionChecksum = await frozenSolution1.State.GetChecksumAsync(CancellationToken.None); + synched = await remoteWorkspace.GetSolutionAsync(assetProvider, solutionChecksum, fromPrimaryBranch: true, workspaceVersion: 1, projectId: null, CancellationToken.None); + Assert.Equal(solutionChecksum, await synched.State.GetChecksumAsync(CancellationToken.None)); + + // Try freezing with some different content from the original solution + var frozenText2 = SourceText.From("// Hello, World! A second time!"); + var frozenSolution2 = solution.WithFrozenSourceGeneratedDocument(documentIdentity, frozenText2).Project.Solution; + + assetProvider = await GetAssetProviderAsync(workspace, remoteWorkspace, frozenSolution2); + solutionChecksum = await frozenSolution2.State.GetChecksumAsync(CancellationToken.None); + synched = await remoteWorkspace.GetSolutionAsync(assetProvider, solutionChecksum, fromPrimaryBranch: true, workspaceVersion: 2, projectId: null, CancellationToken.None); Assert.Equal(solutionChecksum, await synched.State.GetChecksumAsync(CancellationToken.None)); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs index 9422d0778002c..36858b84eb0fe 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs @@ -1758,6 +1758,21 @@ internal Document WithFrozenSourceGeneratedDocument(SourceGeneratedDocumentIdent return newProject.GetOrCreateSourceGeneratedDocument(newDocumentState); } + /// + /// Undoes the operation of ; any frozen source generated document is allowed + /// to have it's real output again. + /// + internal Solution WithoutFrozenSourceGeneratedDocuments() + { + var newState = _state.WithoutFrozenSourceGeneratedDocuments(); + if (newState == _state) + { + return this; + } + + return new Solution(newState); + } + /// /// Gets an objects that lists the added, changed and removed projects between /// this solution and the specified solution. diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index cfa66a9c6a946..84e56211b30c6 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -1908,6 +1908,31 @@ public SolutionState WithFrozenSourceGeneratedDocument(SourceGeneratedDocumentId frozenSourceGeneratedDocument: newGeneratedState); } + /// + /// Undoes the operation of ; any frozen source generated document is allowed + /// to have it's real output again. + /// + public SolutionState WithoutFrozenSourceGeneratedDocuments() + { + // If there's nothing frozen, there's nothing to do. + if (_frozenSourceGeneratedDocumentState == null) + return this; + + var projectId = _frozenSourceGeneratedDocumentState.Id.ProjectId; + + // Since we previously froze this document, we should have a CompilationTracker entry for it, and it should be a + // GeneratedFileReplacingCompilationTracker. To undo the operation, we'll just restore the original CompilationTracker. + var newTrackerMap = CreateCompilationTrackerMap(projectId, _dependencyGraph); + Contract.ThrowIfFalse(newTrackerMap.TryGetValue(projectId, out var existingTracker)); + var replacingItemTracker = existingTracker as GeneratedFileReplacingCompilationTracker; + Contract.ThrowIfNull(replacingItemTracker); + newTrackerMap = newTrackerMap.SetItem(projectId, replacingItemTracker.UnderlyingTracker); + + return this.Branch( + projectIdToTrackerMap: newTrackerMap, + frozenSourceGeneratedDocument: null); + } + /// /// Symbols need to be either or . /// diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index e650ef41b4af7..4e653a4220e18 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -57,6 +57,10 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum) { var solution = _baseSolution; + // If we previously froze a source generated document and then held onto that, unfreeze it now. We'll re-freeze the new document + // if needed again later. + solution = solution.WithoutFrozenSourceGeneratedDocuments(); + var oldSolutionChecksums = await solution.State.GetStateChecksumsAsync(_cancellationToken).ConfigureAwait(false); var newSolutionChecksums = await _assetProvider.GetAssetAsync(newSolutionChecksum, _cancellationToken).ConfigureAwait(false); @@ -85,10 +89,6 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum) newSolutionChecksums.AnalyzerReferences, _cancellationToken).ConfigureAwait(false)); } - // The old solution should never have any frozen source generated documents -- those are only created and forked off of - // a workspaces's CurrentSolution - Contract.ThrowIfFalse(solution.State.FrozenSourceGeneratedDocumentState == null); - if (newSolutionChecksums.FrozenSourceGeneratedDocumentIdentity != Checksum.Null && newSolutionChecksums.FrozenSourceGeneratedDocumentText != Checksum.Null) { var identity = await _assetProvider.GetAssetAsync(newSolutionChecksums.FrozenSourceGeneratedDocumentIdentity, _cancellationToken).ConfigureAwait(false);