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 - Report diagnostic when compiler synthesizes new types on unsupported runtime #55876

Merged
merged 3 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -1659,6 +1659,44 @@ public async Task Capabilities()
EndDebuggingSession(debuggingSession);
}

[Fact]
public async Task Capabilities_SynthesizedNewType()
{
var source1 = "class C { void M() { } }";
var source2 = "class C { void M() { var x = new { Goo = 1 }; } }";
davidwengier marked this conversation as resolved.
Show resolved Hide resolved

using var _ = CreateWorkspace(out var solution, out var service);
solution = AddDefaultTestProject(solution, new[] { source1 });
var project = solution.Projects.Single();
solution = solution.WithProjectParseOptions(project.Id, new CSharpParseOptions(LanguageVersion.CSharp10));
var documentId = solution.Projects.Single().Documents.Single().Id;

EmitAndLoadLibraryToDebuggee(source1);

// attached to processes that doesn't allow creating new types
_debuggerService.GetCapabilitiesImpl = () => ImmutableArray.Create("Baseline");

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

// update document:
solution = solution.WithDocumentText(documentId, SourceText.From(source2, Encoding.UTF8));
var document2 = solution.Projects.Single().Documents.Single();

// These errors aren't reported as document diagnostics
var diagnostics = await service.GetDocumentDiagnosticsAsync(solution.GetDocument(documentId), s_noActiveSpans, CancellationToken.None);
AssertEx.Empty(diagnostics);

// They are reported as emit diagnostics
var (updates, emitDiagnostics) = await EmitSolutionUpdateAsync(debuggingSession, solution);
AssertEx.Equal(new[] { $"{document2.Project.Id} Error ENC1007: {FeaturesResources.ChangesRequiredSynthesizedType}" }, InspectDiagnostics(emitDiagnostics));

// no emitted delta:
Assert.Empty(updates.Updates);

EndDebuggingSession(debuggingSession);
}

[Fact]
public async Task ValidSignificantChange_EmitError()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ void AddGeneralDiagnostic(EditAndContinueErrorCode code, string resourceName, Di
AddGeneralDiagnostic(EditAndContinueErrorCode.ChangesDisallowedWhileStoppedAtException, nameof(FeaturesResources.ChangesDisallowedWhileStoppedAtException));
AddGeneralDiagnostic(EditAndContinueErrorCode.DocumentIsOutOfSyncWithDebuggee, nameof(FeaturesResources.DocumentIsOutOfSyncWithDebuggee), DiagnosticSeverity.Warning);
AddGeneralDiagnostic(EditAndContinueErrorCode.UnableToReadSourceFileOrPdb, nameof(FeaturesResources.UnableToReadSourceFileOrPdb), DiagnosticSeverity.Warning);
AddGeneralDiagnostic(EditAndContinueErrorCode.ChangeResultsInSynthesizedType, nameof(FeaturesResources.ChangesRequiredSynthesizedType));

s_descriptors = builder.ToImmutable();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ internal enum EditAndContinueErrorCode
ChangesDisallowedWhileStoppedAtException = 4,
DocumentIsOutOfSyncWithDebuggee = 5,
UnableToReadSourceFileOrPdb = 6,
ChangeResultsInSynthesizedType = 7,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChangeResultsInSynthesizedType

The name does not indicate that it's runtime-specific limitation. Maybe RuntimeMustSupportAddingTypes or AddingTypeRuntimeCapabilityRequired?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like AddingTypeRuntimeCapabilityRequired, thanks

}
}
41 changes: 40 additions & 1 deletion src/Features/Core/Portable/EditAndContinue/EditSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,40 @@ internal EditSession(
Analyses = new EditAndContinueDocumentAnalysesCache(BaseActiveStatements, Capabilities);
}

/// <summary>
/// The compiler has various scenarios that will cause it to synthesize things that might not be covered
/// by existing rude edits, but we still need to ensure the runtime supports them before we proceed.
/// </summary>
private async Task<Diagnostic?> GetUnsupportedChangesDiagnosticAsync(EmitDifferenceResult emitResult, CancellationToken cancellationToken)
{
if (!emitResult.Success)
{
return null;
}

if (emitResult.Baseline is null)
{
return null;
}

var capabilities = await Capabilities.GetValueAsync(cancellationToken).ConfigureAwait(false);
if (!capabilities.HasFlag(EditAndContinueCapabilities.NewTypeDefinition))
{
// If the runtime doesn't support adding new types then we expect every row number for any type that is
// emitted will be less than or equal to the number of rows in the original metadata.
var highestEmittedTypeDefRow = emitResult.ChangedTypes.Max(t => MetadataTokens.GetRowNumber(t));
var highestExistingTypeDefRow = emitResult.Baseline.OriginalMetadata.GetMetadataReader().GetTableRowCount(TableIndex.TypeDef);

if (highestEmittedTypeDefRow > highestExistingTypeDefRow)
{
var descriptor = EditAndContinueDiagnosticDescriptors.GetDescriptor(EditAndContinueErrorCode.ChangeResultsInSynthesizedType);
return Diagnostic.Create(descriptor, Location.None);
}
}

return null;
}

/// <summary>
/// Errors to be reported when a project is updated but the corresponding module does not support EnC.
/// </summary>
Expand Down Expand Up @@ -834,7 +868,12 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(Solution solution
cancellationToken);
}

if (emitResult.Success)
var unsupportedChangesDiagnostic = await GetUnsupportedChangesDiagnosticAsync(emitResult, cancellationToken).ConfigureAwait(false);
if (unsupportedChangesDiagnostic is not null)
{
diagnostics.Add((newProject.Id, ImmutableArray.Create(unsupportedChangesDiagnostic)));
}
else if (emitResult.Success)
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
{
Contract.ThrowIfNull(emitResult.Baseline);

Expand Down
3 changes: 3 additions & 0 deletions src/Features/Core/Portable/FeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2920,4 +2920,7 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
<data name="Renaming_0_requires_restarting_the_application_because_it_is_not_supported_by_the_runtime" xml:space="preserve">
<value>Renaming {0} requires restarting the application because it is not supported by the runtime.</value>
</data>
<data name="ChangesRequiredSynthesizedType" xml:space="preserve">
<value>One or more changes result in a new type being created by the compiler, which requires restarting the application because it is not supported by the runtime</value>
</data>
</root>
5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading