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

EnC: A couple of small fixes #62454

Merged
merged 4 commits into from
Jul 13, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public async Task TestOpenFileOnlyAnalyzerDiagnostics()
[Fact]
public async Task TestSynchronizeWithBuild()
{
using var workspace = CreateWorkspace(new[] { typeof(NoCompilationLanguageServiceFactory) });
using var workspace = CreateWorkspace(new[] { typeof(NoCompilationLanguageService) });

var analyzerReference = new AnalyzerImageReference(ImmutableArray.Create<DiagnosticAnalyzer>(new NoNameAnalyzer()));
workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { analyzerReference }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ public async Task StartDebuggingSession_CapturingDocuments(bool captureAllDocume
sourceFileB.WriteAllText(sourceB2, encodingB);

// prepare workspace as if it was loaded from project files:
using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });

var projectP = solution.AddProject("P", "P", LanguageNames.CSharp);
solution = projectP.Solution;
Expand Down Expand Up @@ -531,7 +531,7 @@ public async Task StartDebuggingSession_CapturingDocuments(bool captureAllDocume
AddDocument(CreateDesignTimeOnlyDocument(projectP.Id, name: "dt2.cs", path: "dt2.cs"));

// project that does not support EnC - the contents of documents in this project shouldn't be loaded:
var projectQ = solution.AddProject("Q", "Q", DummyLanguageService.LanguageName);
var projectQ = solution.AddProject("Q", "Q", NoCompilationConstants.LanguageName);
solution = projectQ.Solution;

solution = solution.AddDocument(DocumentInfo.Create(
Expand Down Expand Up @@ -576,7 +576,7 @@ public async Task ProjectNotBuilt()

_mockCompilationOutputsProvider = _ => new MockCompilationOutputs(Guid.Empty);

await StartDebuggingSessionAsync(service, solution);
var debuggingSession = await StartDebuggingSessionAsync(service, solution);

// no changes:
var diagnostics = await service.GetDocumentDiagnosticsAsync(document1, s_noActiveSpans, CancellationToken.None);
Expand All @@ -588,6 +588,14 @@ public async Task ProjectNotBuilt()

diagnostics = await service.GetDocumentDiagnosticsAsync(document2, s_noActiveSpans, CancellationToken.None);
Assert.Empty(diagnostics);

// changes in the project are ignored:
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
Assert.Equal(ManagedModuleUpdateStatus.None, updates.Status);
Assert.Empty(updates.Updates);
Assert.Empty(emitDiagnostics);

EndDebuggingSession(debuggingSession);
}

[Fact]
Expand Down Expand Up @@ -631,8 +639,8 @@ public async Task DifferentDocumentWithSameContent()
[CombinatorialData]
public async Task ProjectThatDoesNotSupportEnC(bool breakMode)
{
using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
var project = solution.AddProject("dummy_proj", "dummy_proj", DummyLanguageService.LanguageName);
using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });
var project = solution.AddProject("dummy_proj", "dummy_proj", NoCompilationConstants.LanguageName);
var document = project.AddDocument("test", SourceText.From("dummy1"));
solution = document.Project.Solution;

Expand Down Expand Up @@ -823,8 +831,11 @@ public async Task ErrorReadingModuleFile(bool breakMode)
expectedErrorMessage = e.Message;
}

var source1 = "class C { void M() { System.Console.WriteLine(1); } }";
var source2 = "class C { void M() { System.Console.WriteLine(2); } }";

using var _w = CreateWorkspace(out var solution, out var service);
(solution, var document1) = AddDefaultTestProject(solution, "class C1 { void M() { System.Console.WriteLine(1); } }");
(solution, var document1) = AddDefaultTestProject(solution, source1);

_mockCompilationOutputsProvider = _ => new CompilationOutputFiles(moduleFile.Path);

Expand All @@ -836,7 +847,7 @@ public async Task ErrorReadingModuleFile(bool breakMode)
}

// change the source:
solution = solution.WithDocumentText(document1.Id, SourceText.From("class C1 { void M() { System.Console.WriteLine(2); } }"));
solution = solution.WithDocumentText(document1.Id, SourceText.From(source2, encoding: Encoding.UTF8));
var document2 = solution.GetDocument(document1.Id);

// error not reported here since it might be intermittent and will be reported if the issue persist when applying the update:
Expand All @@ -848,6 +859,15 @@ public async Task ErrorReadingModuleFile(bool breakMode)
Assert.Empty(updates.Updates);
AssertEx.Equal(new[] { $"{document2.Project.Id}: Error ENC1001: {string.Format(FeaturesResources.ErrorReadingFile, moduleFile.Path, expectedErrorMessage)}" }, InspectDiagnostics(emitDiagnostics));

// correct the error:
var correctedMvid = EmitLibrary(source2);
tmat marked this conversation as resolved.
Show resolved Hide resolved

var (updates2, emitDiagnostics2) = await EmitSolutionUpdateAsync(debuggingSession, solution);
Assert.Equal(ManagedModuleUpdateStatus.Ready, updates2.Status);
Assert.Empty(emitDiagnostics2);

CommitSolutionUpdate(debuggingSession);

if (breakMode)
{
ExitBreakState(debuggingSession);
Expand All @@ -859,17 +879,17 @@ public async Task ErrorReadingModuleFile(bool breakMode)
{
AssertEx.Equal(new[]
{
"Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=1|EmptySessionCount=0|HotReloadSessionCount=0|EmptyHotReloadSessionCount=2",
"Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=True|Capabilities=31|ProjectIdsWithAppliedChanges=",
"Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=1|EmptySessionCount=0|HotReloadSessionCount=0|EmptyHotReloadSessionCount=3",
"Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=True|Capabilities=31|ProjectIdsWithAppliedChanges={00000000-AAAA-AAAA-AAAA-111111111111}",
"Debugging_EncSession_EditSession_EmitDeltaErrorId: SessionId=1|EditSessionId=2|ErrorId=ENC1001"
}, _telemetryLog);
}
else
{
AssertEx.Equal(new[]
{
"Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=0|EmptySessionCount=0|HotReloadSessionCount=1|EmptyHotReloadSessionCount=0",
"Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=False|Capabilities=31|ProjectIdsWithAppliedChanges=",
"Debugging_EncSession: SolutionSessionId={00000000-AAAA-AAAA-AAAA-000000000000}|SessionId=1|SessionCount=0|EmptySessionCount=0|HotReloadSessionCount=1|EmptyHotReloadSessionCount=1",
"Debugging_EncSession_EditSession: SessionId=1|EditSessionId=2|HadCompilationErrors=False|HadRudeEdits=False|HadValidChanges=True|HadValidInsignificantChanges=False|RudeEditsCount=0|EmitDeltaErrorIdCount=1|InBreakState=False|Capabilities=31|ProjectIdsWithAppliedChanges={00000000-AAAA-AAAA-AAAA-111111111111}",
"Debugging_EncSession_EditSession_EmitDeltaErrorId: SessionId=1|EditSessionId=2|ErrorId=ENC1001"
}, _telemetryLog);
}
Expand Down Expand Up @@ -3454,11 +3474,11 @@ public async Task ActiveStatements_SyntaxErrorOrOutOfSyncDocument(bool isOutOfSy
[CombinatorialData]
public async Task ActiveStatements_ForeignDocument(bool withPath, bool designTimeOnly)
{
var composition = FeaturesTestCompositions.Features.AddParts(typeof(DummyLanguageService));
var composition = FeaturesTestCompositions.Features.AddParts(typeof(NoCompilationLanguageService));

using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(DummyLanguageService) });
using var _ = CreateWorkspace(out var solution, out var service, new[] { typeof(NoCompilationLanguageService) });

var project = solution.AddProject("dummy_proj", "dummy_proj", designTimeOnly ? LanguageNames.CSharp : DummyLanguageService.LanguageName);
var project = solution.AddProject("dummy_proj", "dummy_proj", designTimeOnly ? LanguageNames.CSharp : NoCompilationConstants.LanguageName);
var filePath = withPath ? Path.Combine(TempRoot.Root, "test.cs") : null;

var documentInfo = DocumentInfo.Create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.UnitTests;
using Moq;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
Expand All @@ -31,7 +32,7 @@ namespace Microsoft.CodeAnalysis.EditAndContinue.UnitTests
[UseExportProvider]
public class EditSessionActiveStatementsTests : TestBase
{
private static readonly TestComposition s_composition = EditorTestCompositions.EditorFeatures.AddParts(typeof(DummyLanguageService));
private static readonly TestComposition s_composition = EditorTestCompositions.EditorFeatures.AddParts(typeof(NoCompilationLanguageService));

private static EditSession CreateEditSession(
Solution solution,
Expand Down Expand Up @@ -174,8 +175,8 @@ static void Main()

var solution = AddDefaultTestSolution(workspace, markedSources);
var projectId = solution.ProjectIds.Single();
var dummyProject = solution.AddProject("dummy_proj", "dummy_proj", DummyLanguageService.LanguageName);
solution = dummyProject.Solution.AddDocument(DocumentId.CreateNewId(dummyProject.Id, DummyLanguageService.LanguageName), "a.dummy", "");
var dummyProject = solution.AddProject("dummy_proj", "dummy_proj", NoCompilationConstants.LanguageName);
solution = dummyProject.Solution.AddDocument(DocumentId.CreateNewId(dummyProject.Id, NoCompilationConstants.LanguageName), "a.dummy", "");
var project = solution.GetProject(projectId);
var document1 = project.Documents.Single(d => d.Name == "test1.cs");
var document2 = project.Documents.Single(d => d.Name == "test2.cs");
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public async Task<(Document? Document, DocumentState State)> GetDocumentAndState
var (project, documentStates) = projectDocumentStates;

// Skip projects that do not support Roslyn EnC (e.g. F#, etc).
// Source files of these do not even need to be captured in the solution snapshot.
// Source files of these may not even be captured in the solution snapshot.
if (!project.SupportsEditAndContinue())
{
return Array.Empty<DocumentId?>();
Expand Down
34 changes: 28 additions & 6 deletions src/Features/Core/Portable/EditAndContinue/DebuggingSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal sealed class DebuggingSession : IDisposable

/// <summary>
/// MVIDs read from the assembly built for given project id.
/// Only contains ids for projects that support EnC.
/// </summary>
private readonly Dictionary<ProjectId, (Guid Mvid, Diagnostic Error)> _projectModuleIds = new();
private readonly Dictionary<Guid, ProjectId> _moduleIds = new();
Expand Down Expand Up @@ -241,10 +242,12 @@ private bool AddModulePreparedForUpdate(Guid mvid)
/// </summary>
/// <returns>
/// An MVID and an error message to report, in case an IO exception occurred while reading the binary.
/// The MVID is default if either project not built, or an it can't be read from the module binary.
/// The MVID is <see cref="Guid.Empty"/> if either the project is not built, or the MVID can't be read from the module binary.
/// </returns>
internal async Task<(Guid Mvid, Diagnostic? Error)> GetProjectModuleIdAsync(Project project, CancellationToken cancellationToken)
{
Debug.Assert(project.SupportsEditAndContinue());

lock (_projectModuleIdsGuard)
{
if (_projectModuleIds.TryGetValue(project.Id, out var id))
Expand Down Expand Up @@ -281,8 +284,14 @@ internal async Task<(Guid Mvid, Diagnostic? Error)> GetProjectModuleIdAsync(Proj
return id;
}

_moduleIds[newId.Mvid] = project.Id;
return _projectModuleIds[project.Id] = newId;
// Do not cache failures. The error might be intermittent and might be corrected next time we attempt to read the MVID.
if (newId.Mvid != Guid.Empty)
{
_moduleIds[newId.Mvid] = project.Id;
_projectModuleIds[project.Id] = newId;
}

return newId;
}
}

Expand Down Expand Up @@ -650,6 +659,10 @@ public async ValueTask<ImmutableArray<ImmutableArray<ActiveStatementSpan>>> GetB
}

var newProject = solution.GetRequiredProject(projectId);

Debug.Assert(oldProject.SupportsEditAndContinue());
Debug.Assert(newProject.SupportsEditAndContinue());

var analyzer = newProject.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();

await foreach (var documentId in EditSession.GetChangedDocumentsAsync(oldProject, newProject, cancellationToken).ConfigureAwait(false))
Expand Down Expand Up @@ -745,7 +758,7 @@ public async ValueTask<ImmutableArray<ActiveStatementSpan>> GetAdjustedActiveSta
{
try
{
if (_isDisposed || !EditSession.InBreakState || !mappedDocument.State.SupportsEditAndContinue())
if (_isDisposed || !EditSession.InBreakState || !mappedDocument.State.SupportsEditAndContinue() || !mappedDocument.Project.SupportsEditAndContinue())
tmat marked this conversation as resolved.
Show resolved Hide resolved
{
return ImmutableArray<ActiveStatementSpan>.Empty;
}
Expand Down Expand Up @@ -965,6 +978,10 @@ await foreach (var unmappedDocumentId in EditSession.GetChangedDocumentsAsync(ol
return null;
}

// We only maintain module ids for projects that support EnC:
Debug.Assert(oldProject.SupportsEditAndContinue());
Debug.Assert(newProject.SupportsEditAndContinue());

documentId = await GetChangedDocumentContainingUnmappedActiveStatementAsync(activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, cancellationToken).ConfigureAwait(false);
}
else
Expand All @@ -982,8 +999,10 @@ async Task GetTaskAsync(ProjectId projectId)
// TODO: https://github.com/dotnet/roslyn/issues/1204
// oldProject == null ==> project has been added - it may have active statements if the project was unloaded when debugging session started but the sources
// correspond to the PDB.
var id = (oldProject != null) ? await GetChangedDocumentContainingUnmappedActiveStatementAsync(
activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, linkedTokenSource.Token).ConfigureAwait(false) : null;
var id = (oldProject?.SupportsEditAndContinue() == true) ?
await GetChangedDocumentContainingUnmappedActiveStatementAsync(
activeStatementsMap, LastCommittedSolution, oldProject, newProject, baseActiveStatement, linkedTokenSource.Token).ConfigureAwait(false) :
null;

Interlocked.CompareExchange(ref documentId, id, null);
if (id != null)
Expand Down Expand Up @@ -1017,6 +1036,9 @@ async Task GetTaskAsync(ProjectId projectId)
private static async ValueTask<DocumentId?> GetChangedDocumentContainingUnmappedActiveStatementAsync(ActiveStatementsMap baseActiveStatements, CommittedSolution oldSolution, Project oldProject, Project newProject, ActiveStatement activeStatement, CancellationToken cancellationToken)
{
Debug.Assert(oldProject.Id == newProject.Id);
Debug.Assert(oldProject.SupportsEditAndContinue());
Debug.Assert(newProject.SupportsEditAndContinue());

var analyzer = newProject.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();

await foreach (var documentId in EditSession.GetChangedDocumentsAsync(oldProject, newProject, cancellationToken).ConfigureAwait(false))
Expand Down
4 changes: 4 additions & 0 deletions src/Features/Core/Portable/EditAndContinue/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public static UnmappedActiveStatement GetStatement(this ImmutableArray<UnmappedA
throw ExceptionUtilities.UnexpectedValue(ordinal);
}

/// <summary>
/// True if the project supports Edit and Continue.
/// Only depends on the language of the project and never changes.
/// </summary>
public static bool SupportsEditAndContinue(this Project project)
=> project.LanguageServices.GetService<IEditAndContinueAnalyzer>() != null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public async Task RemoveProjectConvertsProjectReferencesBack()
[Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
public async Task AddingMetadataReferenceToProjectThatCannotCompileInTheIdeKeepsMetadataReference()
{
using var environment = new TestEnvironment(typeof(NoCompilationLanguageServiceFactory));
using var environment = new TestEnvironment(typeof(NoCompilationLanguageService));
var project1 = await CreateCSharpCPSProjectAsync(environment, "project1", commandLineArguments: @"/out:c:\project1.dll");
var project2 = await CreateNonCompilableProjectAsync(environment, "project2", @"C:\project2.fsproj");
project2.BinOutputPath = "c:\\project2.dll";
Expand Down
Loading