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] Allow deleting method #61806

Merged
merged 44 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
0495051
Synthesize a method that throws when a method is deleted
davidwengier Apr 29, 2022
cedfe6f
Fix
davidwengier May 3, 2022
b960ebd
Expand test
davidwengier May 3, 2022
7623710
Try a different approach
davidwengier May 6, 2022
b6e3c10
Make the test work!
davidwengier May 12, 2022
11b261a
Use NewSymbol to track containing type of deleted symbols
davidwengier May 19, 2022
adfba9c
Various renames and doc updates
davidwengier May 19, 2022
dd6a25e
Add a delete edit when a method is deleted
davidwengier May 23, 2022
16c0f7f
Fix build
davidwengier May 23, 2022
015c4d3
Fix VB tests
davidwengier May 24, 2022
1737ec8
Wrap parameter and method return types
davidwengier Jun 7, 2022
4689c6c
Fix
davidwengier Jun 7, 2022
263dfa1
Cache type definitions used by deleted members
davidwengier Jun 8, 2022
1c75ca5
Allow deleting a member that was previously added
davidwengier Jun 8, 2022
9409a4f
When re-adding a method, treat it as an update
davidwengier Jun 8, 2022
4091b60
Better exceptions
davidwengier Jun 9, 2022
411d867
Revert
davidwengier Jun 9, 2022
690093f
Test across multiple generations
davidwengier Jun 9, 2022
2206588
Fix type symbol matching for method return types
davidwengier Jun 9, 2022
31c3a10
Support attributes on deleted methods and parameters
davidwengier Jun 9, 2022
19db170
Fix VB tests
davidwengier Jun 10, 2022
5de5662
Restore assert
davidwengier Jun 10, 2022
ab8a3a2
VB test
davidwengier Jun 10, 2022
8b2eafb
Fix method handling
davidwengier Jun 10, 2022
500252c
Delete debugging code
davidwengier Jun 10, 2022
dec3448
Emit an actual exception
davidwengier Jun 10, 2022
2047f27
Fix replaced types
davidwengier Jun 14, 2022
a818baa
Fix nullable warnings
davidwengier Jun 14, 2022
e96fe17
Use the correct exception type
davidwengier Jun 14, 2022
4316266
Fix
davidwengier Jun 14, 2022
253bae5
Wrap generic parameter definitions
davidwengier Jun 14, 2022
ed4ae29
Fix nullable warnings
davidwengier Jun 15, 2022
d2c5883
PR feedback
davidwengier Jun 17, 2022
c890cda
nullability
davidwengier Jun 17, 2022
e5845ec
Report rude edit for virtual and abstract methods
davidwengier Jun 17, 2022
4db502e
Change semantic edit info storage
davidwengier Jun 20, 2022
a35e49b
PR feedback
davidwengier Jun 22, 2022
6a2c709
Fix
davidwengier Jun 22, 2022
5f80586
Fix
davidwengier Jun 22, 2022
ee42237
Fix
davidwengier Jun 22, 2022
9acc95c
Fix
davidwengier Jun 22, 2022
cc55181
Remove unused using
davidwengier Jun 24, 2022
3542028
Merge remote-tracking branch 'upstream/main' into EnCDeleteMethod
davidwengier Jun 27, 2022
b647db7
Cleanup
davidwengier Jun 28, 2022
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 @@ -32,10 +32,11 @@ public CSharpSymbolMatcher(
EmitContext sourceContext,
SourceAssemblySymbol otherAssembly,
EmitContext otherContext,
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> otherSynthesizedMembersOpt)
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherSynthesizedMembers,
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherDeletedMembers)
{
_defs = new MatchDefsToSource(sourceContext, otherContext);
_symbols = new MatchSymbols(anonymousTypeMap, anonymousDelegates, anonymousDelegatesWithFixedTypes, sourceAssembly, otherAssembly, otherSynthesizedMembersOpt, new DeepTranslator(otherAssembly.GetSpecialType(SpecialType.System_Object)));
_symbols = new MatchSymbols(anonymousTypeMap, anonymousDelegates, anonymousDelegatesWithFixedTypes, sourceAssembly, otherAssembly, otherSynthesizedMembers, otherDeletedMembers, new DeepTranslator(otherAssembly.GetSpecialType(SpecialType.System_Object)));
}

public CSharpSymbolMatcher(
Expand All @@ -55,7 +56,8 @@ public CSharpSymbolMatcher(
sourceAssembly,
otherAssembly,
otherSynthesizedMembers: null,
deepTranslator: null);
deepTranslator: null,
otherDeletedMembers: null);
}

public override Cci.IDefinition? MapDefinition(Cci.IDefinition definition)
Expand Down Expand Up @@ -292,6 +294,8 @@ private sealed class MatchSymbols : CSharpSymbolVisitor<Symbol?>
/// </summary>
private readonly ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? _otherSynthesizedMembers;

private readonly ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? _otherDeletedMembers;

private readonly SymbolComparer _comparer;
private readonly ConcurrentDictionary<Symbol, Symbol?> _matches = new(ReferenceEqualityComparer.Instance);

Expand All @@ -310,6 +314,7 @@ public MatchSymbols(
SourceAssemblySymbol sourceAssembly,
AssemblySymbol otherAssembly,
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherSynthesizedMembers,
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherDeletedMembers,
DeepTranslator? deepTranslator)
{
_anonymousTypeMap = anonymousTypeMap;
Expand All @@ -318,6 +323,7 @@ public MatchSymbols(
_sourceAssembly = sourceAssembly;
_otherAssembly = otherAssembly;
_otherSynthesizedMembers = otherSynthesizedMembers;
_otherDeletedMembers = otherDeletedMembers;
_comparer = new SymbolComparer(this, deepTranslator);
}

Expand Down Expand Up @@ -987,6 +993,11 @@ private IReadOnlyDictionary<string, ImmutableArray<ISymbolInternal>> GetAllEmitt
members.AddRange(synthesizedMembers);
}

if (_otherDeletedMembers?.TryGetValue(symbol, out var deletedMembers) == true)
{
members.AddRange(deletedMembers);
}

var result = members.ToDictionary(s => s.MetadataName, StringOrdinalComparer.Instance);
members.Free();
return result;
Expand All @@ -1006,6 +1017,11 @@ public SymbolComparer(MatchSymbols matcher, DeepTranslator? deepTranslator)

public bool Equals(TypeSymbol source, TypeSymbol other)
{
if (ReferenceEquals(source, other))
{
return true;
}

var visitedSource = (TypeSymbol?)_matcher.Visit(source);
var visitedOther = (_deepTranslator != null) ? (TypeSymbol)_deepTranslator.Visit(other) : other;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,10 @@ private static EmitBaseline MapToCompilation(

RoslynDebug.AssertNotNull(previousGeneration.Compilation);
RoslynDebug.AssertNotNull(previousGeneration.PEModuleBuilder);
RoslynDebug.AssertNotNull(moduleBeingBuilt.EncSymbolChanges);

var currentSynthesizedMembers = moduleBeingBuilt.GetAllSynthesizedMembers();
var currentDeletedMembers = moduleBeingBuilt.EncSymbolChanges.GetAllDeletedMethods();

// Mapping from previous compilation to the current.
var anonymousTypeMap = moduleBeingBuilt.GetAnonymousTypeMap();
Expand All @@ -193,10 +195,14 @@ private static EmitBaseline MapToCompilation(
sourceContext,
compilation.SourceAssembly,
otherContext,
currentSynthesizedMembers);
currentSynthesizedMembers,
currentDeletedMembers);

var mappedSynthesizedMembers = matcher.MapSynthesizedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers);

// Deleted members are mapped the same way as synthesized members, so we can just call the same method.
var mappedDeletedMembers = matcher.MapSynthesizedMembers(previousGeneration.DeletedMembers, currentDeletedMembers);

// TODO: can we reuse some data from the previous matcher?
var matcherWithAllSynthesizedMembers = new CSharpSymbolMatcher(
anonymousTypeMap,
Expand All @@ -206,13 +212,15 @@ private static EmitBaseline MapToCompilation(
sourceContext,
compilation.SourceAssembly,
otherContext,
mappedSynthesizedMembers);
mappedSynthesizedMembers,
mappedDeletedMembers);

return matcherWithAllSynthesizedMembers.MapBaselineToCompilation(
previousGeneration,
compilation,
moduleBeingBuilt,
mappedSynthesizedMembers);
mappedSynthesizedMembers,
mappedDeletedMembers);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ public PEDeltaAssemblyBuilder(
sourceContext: context,
otherAssembly: previousAssembly,
otherContext: previousContext,
otherSynthesizedMembersOpt: previousGeneration.SynthesizedMembers);
otherSynthesizedMembers: previousGeneration.SynthesizedMembers,
otherDeletedMembers: previousGeneration.DeletedMembers);
}

_previousDefinitions = new CSharpDefinitionMap(edits, metadataDecoder, matchToMetadata, matchToPrevious);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Reflection.Metadata;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.EditAndContinue.UnitTests
{
Expand All @@ -16,13 +17,15 @@ internal sealed class GenerationInfo
public readonly MetadataReader MetadataReader;
public readonly EmitBaseline Baseline;
public readonly Action<GenerationVerifier> Verifier;
public readonly CompilationDifference? CompilationDifference;

public GenerationInfo(CSharpCompilation compilation, MetadataReader reader, EmitBaseline baseline, Action<GenerationVerifier> verifier)
public GenerationInfo(CSharpCompilation compilation, MetadataReader reader, CompilationDifference? diff, EmitBaseline baseline, Action<GenerationVerifier> verifier)
{
Compilation = compilation;
MetadataReader = reader;
Baseline = baseline;
Verifier = verifier;
CompilationDifference = diff;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;
using Microsoft.CodeAnalysis.CSharp.UnitTests;
using Roslyn.Test.Utilities;
using static Microsoft.CodeAnalysis.CSharp.EditAndContinue.UnitTests.EditAndContinueTestBase;

Expand All @@ -18,12 +19,14 @@ internal sealed class GenerationVerifier
private readonly int _ordinal;
private readonly MetadataReader _metadataReader;
private readonly IEnumerable<MetadataReader> _readers;
private readonly GenerationInfo _generationInfo;

public GenerationVerifier(int ordinal, MetadataReader metadataReader, IEnumerable<MetadataReader> readers)
public GenerationVerifier(int ordinal, GenerationInfo generationInfo, IEnumerable<MetadataReader> readers)
{
_ordinal = ordinal;
_metadataReader = metadataReader;
_metadataReader = generationInfo.MetadataReader;
_readers = readers;
_generationInfo = generationInfo;
}

private string GetAssertMessage(string message)
Expand Down Expand Up @@ -91,6 +94,11 @@ internal void VerifyCustomAttributes(IEnumerable<CustomAttributeRow> expected)
{
AssertEx.Equal(expected, _metadataReader.GetCustomAttributeRows(), itemInspector: AttributeRowToString);
}

internal void VerifyIL(string expectedIL)
{
_generationInfo.CompilationDifference!.VerifyIL(expectedIL);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ internal EditAndContinueTest AddGeneration(string source, Action<GenerationVerif

var baseline = EmitBaseline.CreateInitialBaseline(md, EditAndContinueTestBase.EmptyLocalsProvider);

_generations.Add(new GenerationInfo(compilation, md.MetadataReader, baseline, validator));
_generations.Add(new GenerationInfo(compilation, md.MetadataReader, diff: null, baseline, validator));

return this;
}
Expand All @@ -62,12 +62,20 @@ internal EditAndContinueTest AddGeneration(string source, SemanticEditDescriptio

var semanticEdits = GetSemanticEdits(edits, prevGeneration.Compilation, compilation);

var diff = compilation.EmitDifference(prevGeneration.Baseline, semanticEdits);
CompilationDifference diff;
try
{
diff = compilation.EmitDifference(prevGeneration.Baseline, semanticEdits);
}
catch (Exception ex)
{
throw new Exception($"Exception during generation #{_generations.Count}. See inner stack trace for details.", ex);
}

var md = diff.GetMetadata();
_disposables.Add(md);

_generations.Add(new GenerationInfo(compilation, md.Reader, diff.NextGeneration, validator));
_generations.Add(new GenerationInfo(compilation, md.Reader, diff, diff.NextGeneration, validator));

return this;
}
Expand All @@ -88,7 +96,7 @@ internal EditAndContinueTest Verify()
}

readers.Add(generation.MetadataReader);
var verifier = new GenerationVerifier(index, generation.MetadataReader, readers);
var verifier = new GenerationVerifier(index, generation, readers);
generation.Verifier(verifier);

index++;
Expand All @@ -99,7 +107,7 @@ internal EditAndContinueTest Verify()

private ImmutableArray<SemanticEdit> GetSemanticEdits(SemanticEditDescription[] edits, Compilation oldCompilation, Compilation newCompilation)
{
return ImmutableArray.CreateRange(edits.Select(e => new SemanticEdit(e.Kind, e.SymbolProvider(oldCompilation), e.SymbolProvider(newCompilation))));
return ImmutableArray.CreateRange(edits.Select(e => new SemanticEdit(e.Kind, e.SymbolProvider(oldCompilation), e.NewSymbolProvider(newCompilation))));
}

public void Dispose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ internal static StatementSyntax GetNearestStatement(SyntaxNode node)
return null;
}

internal static SemanticEditDescription Edit(SemanticEditKind kind, Func<Compilation, ISymbol> symbolProvider)
=> new(kind, symbolProvider);
internal static SemanticEditDescription Edit(SemanticEditKind kind, Func<Compilation, ISymbol> symbolProvider, Func<Compilation, ISymbol> newSymbolProvider = null)
=> new(kind, symbolProvider, newSymbolProvider);

internal static EditAndContinueLogEntry Row(int rowNumber, TableIndex table, EditAndContinueOperation operation)
{
Expand Down
Loading