Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge release/dev17.12 to release/dev17.13 #75835

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Features/CSharp/Portable/Debugging/DataTipInfoGetter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ internal static async Task<DebugDataTipInfo> GetInfoAsync(Document document, int

if (expression is LiteralExpressionSyntax)
{
// If the user hovers over a literal, give them a DataTip for the type of the
// literal they're hovering over.
// Partial semantics should always be sufficient because the (unconverted) type
// of a literal can always easily be determined.
var (_, semanticModel) = await document.GetFullOrPartialSemanticModelAsync(cancellationToken).ConfigureAwait(false);
// If the user hovers over a literal, give them a DataTip for the type of the literal they're hovering
// over. Partial semantics should always be sufficient because the (unconverted) type of a literal can
// always easily be determined.
document = document.WithFrozenPartialSemantics(cancellationToken);
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type;
return type == null
? default
Expand Down
6 changes: 4 additions & 2 deletions src/Features/Core/Portable/Completion/CompletionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ public virtual TextSpan GetDefaultCompletionListSpan(SourceText text, int caretP
var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService<IExtensionManager>();

// We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it.
(document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken);
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var description = await extensionManager.PerformFunctionAsync(
provider,
cancellationToken => provider.GetDescriptionAsync(document, item, options, displayOptions, cancellationToken),
Expand Down Expand Up @@ -246,7 +247,8 @@ public virtual async Task<CompletionChange> GetChangeAsync(
var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService<IExtensionManager>();

// We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it.
(document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken);
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

var change = await extensionManager.PerformFunctionAsync(
provider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ internal virtual async Task<CompletionList> GetCompletionsAsync(
CancellationToken cancellationToken = default)
{
// We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it.
(document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken);
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
var completionListSpan = GetDefaultCompletionListSpan(text, caretPosition);
Expand Down Expand Up @@ -177,14 +178,12 @@ static async Task<ImmutableArray<CompletionProvider>> GetAugmentingProvidersAsyn
/// In most cases we'd still end up with complete document, but we'd consider it an acceptable trade-off even when
/// we get into this transient state.
/// </summary>
private async Task<(Document document, SemanticModel? semanticModel)> GetDocumentWithFrozenPartialSemanticsAsync(Document document, CancellationToken cancellationToken)
private Document GetDocumentWithFrozenPartialSemantics(Document document, CancellationToken cancellationToken)
{
if (_suppressPartialSemantics)
{
return (document, await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false));
}
return document;

return await document.GetFullOrPartialSemanticModelAsync(cancellationToken).ConfigureAwait(false);
return document.WithFrozenPartialSemantics(cancellationToken);
}

private static bool ValidatePossibleTriggerCharacterSet(CompletionTriggerKind completionTriggerKind, IEnumerable<CompletionProvider> triggeredProviders,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ private async Task<Document> DetermineNewDocumentAsync(
var tree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var token = GetToken(completionItem, tree, cancellationToken);
var annotatedRoot = tree.GetRoot(cancellationToken).ReplaceToken(token, token.WithAdditionalAnnotations(_otherAnnotation));
// Make sure the new document is frozen before we try to get the semantic model. This is to
// avoid trigger source generator, which is expensive and not needed for calculating the change.
document = document.WithSyntaxRoot(annotatedRoot).WithFrozenPartialSemantics(cancellationToken);

// Make sure the new document is frozen before we try to get the semantic model. This is to avoid trigger source
// generator, which is expensive and not needed for calculating the change. Pass in 'forceFreeze: true' to
// ensure all further transformations we make do not run generators either.
document = document.WithSyntaxRoot(annotatedRoot).WithFrozenPartialSemantics(forceFreeze: true, cancellationToken);

var memberContainingDocument = await GenerateMemberAndUsingsAsync(document, completionItem, line, cancellationToken).ConfigureAwait(false);
if (memberContainingDocument == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,10 @@ public override async Task ProvideCompletionsAsync(CompletionContext context)
var textChange = new TextChange(span, string.Empty);
originalText = originalText.WithChanges(textChange);

// The document might not be frozen, so make sure we freeze it here to avoid triggering source generator
// which is not needed for snippet completion and will cause perf issue.
var newDocument = document.WithText(originalText).WithFrozenPartialSemantics(cancellationToken);
// The document might not be frozen, so make sure we freeze it here to avoid triggering source generator which
// is not needed for snippet completion and will cause perf issue. Pass in 'forceFreeze: true' to ensure all
// further transformations we make do not run generators either.
var newDocument = document.WithText(originalText).WithFrozenPartialSemantics(forceFreeze: true, cancellationToken);
return (newDocument, span.Start);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.FindSymbols;
Expand All @@ -23,28 +22,6 @@ public static async ValueTask<SyntaxTreeIndex> GetSyntaxTreeIndexAsync(this Docu
public static ValueTask<SyntaxTreeIndex?> GetSyntaxTreeIndexAsync(this Document document, bool loadOnly, CancellationToken cancellationToken)
=> SyntaxTreeIndex.GetIndexAsync(document, loadOnly, cancellationToken);

/// <summary>
/// Returns the semantic model for this document that may be produced from partial semantics. The semantic model
/// is only guaranteed to contain the syntax tree for <paramref name="document"/> and nothing else.
/// </summary>
public static async Task<(Document document, SemanticModel? semanticModel)> GetFullOrPartialSemanticModelAsync(this Document document, CancellationToken cancellationToken)
{
if (document.Project.TryGetCompilation(out var compilation))
{
// We already have a compilation, so at this point it's fastest to just get a SemanticModel
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

// Make sure the compilation is kept alive so that GetSemanticModelAsync() doesn't become expensive
GC.KeepAlive(compilation);
return (document, semanticModel);
}
else
{
var frozenDocument = document.WithFrozenPartialSemantics(cancellationToken);
return (frozenDocument, await frozenDocument.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false));
}
}

internal static Document WithSolutionOptions(this Document document, OptionSet options)
=> document.Project.Solution.WithOptions(options).GetRequiredDocument(document.Id);
}
31 changes: 24 additions & 7 deletions src/Workspaces/Core/Portable/Workspace/Solution/Document.cs
Original file line number Diff line number Diff line change
Expand Up @@ -487,18 +487,35 @@ public ImmutableArray<DocumentId> GetLinkedDocumentIds()
return filteredDocumentIds.Remove(this.Id);
}

/// <inheritdoc cref="WithFrozenPartialSemantics(bool, CancellationToken)"/>
internal Document WithFrozenPartialSemantics(CancellationToken cancellationToken)
=> WithFrozenPartialSemantics(forceFreeze: false, cancellationToken);

/// <summary>
/// Creates a branched version of this document that has its semantic model frozen in whatever state it is available at the time,
/// assuming a background process is constructing the semantics asynchronously. Repeated calls to this method may return
/// documents with increasingly more complete semantics.
/// Creates a branched version of this document that has its semantic model frozen in whatever state it is available
/// at the time, assuming a background process is constructing the semantics asynchronously. Repeated calls to this
/// method may return documents with increasingly more complete semantics.
/// <para/>
/// Use this method to gain access to potentially incomplete semantics quickly.
/// <para/> Note: this will give back a solution where this <see cref="Document"/>'s project will not run
/// generators when getting its compilation. However, all other projects will still run generators when their
/// compilations are requested.
/// <para/> Note: this will give back a solution where this <see cref="Document"/>'s project will not run generators
/// when getting its compilation. However, all other projects will still run generators when their compilations are
/// requested.
/// </summary>
internal virtual Document WithFrozenPartialSemantics(CancellationToken cancellationToken)
/// <param name="forceFreeze">If <see langword="true"/> then a forked document will be returned no matter what. This
/// should be used when the caller wants to ensure that further forks of that document will remain frozen and will
/// not run generators/skeletons. For example, if it is about to transform the document many times, and is fine with
/// the original semantic information they started with. If <see langword="false"/> then this same document may be
/// returned if the compilation for its <see cref="Project"/> was already produced. In this case, generators and
/// skeletons will already have been run, so returning the same instance will be fast when getting semantics.
/// However, this does mean that future forks of this instance will continue running generators/skeletons. This
/// should be used for most clients that intend to just query for semantic information and do not intend to make any
/// further changes.
/// </param>
internal virtual Document WithFrozenPartialSemantics(bool forceFreeze, CancellationToken cancellationToken)
{
if (!forceFreeze && this.Project.TryGetCompilation(out _))
return this;

var solution = this.Project.Solution;

// only produce doc with frozen semantics if this workspace has support for that, as without
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal SourceGeneratedDocument(Project project, SourceGeneratedDocumentState s

internal DateTime GenerationDateTime => State.GenerationDateTime;

internal override Document WithFrozenPartialSemantics(CancellationToken cancellationToken)
internal override Document WithFrozenPartialSemantics(bool forceFreeze, CancellationToken cancellationToken)
{
// For us to implement frozen partial semantics here with a source generated document,
// we'd need to potentially deal with the combination where that happens on a snapshot that was already
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ static async Task AssertCompilationContainsGeneratedFilesAsync(Project project,
}

[Theory, CombinatorialData]
public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration(TestHost testHost)
public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration(
TestHost testHost, bool forceFreeze)
{
using var workspace = CreateWorkspaceWithPartialSemantics(testHost);
var analyzerReference = new TestGeneratorReference(new GenerateFileForEachAdditionalFileWithContentsCommented());
Expand All @@ -331,8 +332,15 @@ public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration(Te

Assert.Equal(2, fullCompilation.SyntaxTrees.Count());

var partialProject = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None).Project;
Assert.NotSame(partialProject, project);
var partialProject = project.Documents.Single().WithFrozenPartialSemantics(forceFreeze, CancellationToken.None).Project;

// If we're forcing the freeze, we must get a different project instance. If we're not, we'll get the same
// project since the compilation was already available.
if (forceFreeze)
Assert.NotSame(partialProject, project);
else
Assert.Same(partialProject, project);

var partialCompilation = await partialProject.GetRequiredCompilationAsync(CancellationToken.None);

Assert.Same(fullCompilation, partialCompilation);
Expand Down Expand Up @@ -721,7 +729,7 @@ public async Task FreezingSolutionEnsuresGeneratorsDoNotRun(bool forkBeforeFreez
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/56702")]
public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost)
public async Task ForkAfterForceFreezeNoLongerRunsGenerators(TestHost testHost)
{
using var workspace = CreateWorkspaceWithPartialSemantics(testHost);
var generatorRan = false;
Expand All @@ -736,9 +744,10 @@ public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost)
Assert.True(generatorRan);
generatorRan = false;

var document = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None);
var document = project.Documents.Single().WithFrozenPartialSemantics(forceFreeze: true, CancellationToken.None);

// And fork with new contents; we'll ensure the contents of this tree are different, but the generator will still not be ran
// And fork with new contents; we'll ensure the contents of this tree are different, but the generator will not
// run since we explicitly force froze.
document = document.WithText(SourceText.From("// Something else"));

var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None);
Expand All @@ -748,6 +757,35 @@ public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost)
Assert.Equal("// Something else", (await document.GetRequiredSyntaxRootAsync(CancellationToken.None)).ToFullString());
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/56702")]
public async Task ForkAfterFreezeOfCompletedDocumentStillRunsGenerators(TestHost testHost)
{
using var workspace = CreateWorkspaceWithPartialSemantics(testHost);
var generatorRan = false;
var analyzerReference = new TestGeneratorReference(new CallbackGenerator(_ => { }, onExecute: _ => { generatorRan = true; }, source: "// Hello World!"));
var project = AddEmptyProject(workspace.CurrentSolution)
.AddAnalyzerReference(analyzerReference)
.AddDocument("RegularDocument.cs", "// Source File", filePath: "RegularDocument.cs").Project;

// Ensure generators are ran
var objectReference = await project.GetCompilationAsync();

Assert.True(generatorRan);
generatorRan = false;

var document = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None);

// And fork with new contents; we'll ensure the contents of this tree are different, but the generator will run
// since we didn't force freeze, and we got the frozen document after its compilation was already computed.
document = document.WithText(SourceText.From("// Something else"));

var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None);
Assert.Equal(2, compilation.SyntaxTrees.Count());
Assert.True(generatorRan);

Assert.Equal("// Something else", (await document.GetRequiredSyntaxRootAsync(CancellationToken.None)).ToFullString());
}

[Theory, CombinatorialData]
public async Task LinkedDocumentOfFrozenShouldNotRunSourceGenerator(TestHost testHost)
{
Expand Down
Loading