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 renaming methods, properties, events etc. #62364

Merged
merged 12 commits into from
Jul 21, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ internal static class EmitHelpers
currentSynthesizedMembers,
currentDeletedMembers);

var mappedSynthesizedMembers = matcher.MapSynthesizedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers);
var mappedSynthesizedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers, isDeletedMemberMapping: false);

// 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);
var mappedDeletedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.DeletedMembers, currentDeletedMembers, isDeletedMemberMapping: true);

// TODO: can we reuse some data from the previous matcher?
var matcherWithAllSynthesizedMembers = new CSharpSymbolMatcher(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ internal void VerifyPropertyDefNames(params string[] expected)
AssertEx.Equal(expected, actual, message: GetAssertMessage("PropertyDefs don't match"));
}

internal void VerifyDeletedMembers(params string[] expected)
{
var actual = _generationInfo.Baseline.DeletedMembers.Select(e => e.Key.ToString() + ": {" + string.Join(", ", e.Value.Select(v => v.Name)) + "}");
AssertEx.SetEqual(expected, actual, itemSeparator: ",\r\n", itemInspector: s => $"\"{s}\"");
}

internal void VerifyTableSize(TableIndex table, int expected)
{
AssertEx.AreEqual(expected, _metadataReader.GetTableRowCount(table), message: GetAssertMessage($"{table} table size doesnt't match"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Immutable;
using System.Linq;
using System.Reflection.Metadata;
using System.Runtime.InteropServices;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand Down Expand Up @@ -112,7 +113,11 @@ private ImmutableArray<SemanticEdit> GetSemanticEdits(SemanticEditDescription[]

public void Dispose()
{
Assert.True(_hasVerified, "No Verify call since the last AddGeneration call.");
// If the test has thrown an exception, or the test host has crashed, we don't want to assert here
// or we'll hide it, so we need to do this dodgy looking thing.
var isInException = Marshal.GetExceptionPointers() != IntPtr.Zero;

Assert.True(isInException || _hasVerified, "No Verify call since the last AddGeneration call.");
foreach (var disposable in _disposables)
{
disposable.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13624,7 +13624,7 @@ class C
}
""",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M2"), newSymbolProvider: c=>c.GetMember("C")),
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M2"), newSymbolProvider: c => c.GetMember("C")),
},
validator: g =>
{
Expand Down Expand Up @@ -13679,7 +13679,7 @@ class C
}
""",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c=>c.GetMember("C")),
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c => c.GetMember("C")),
},
validator: g =>
{
Expand Down Expand Up @@ -13781,7 +13781,7 @@ class C
}
""",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c=>c.GetMember("C")),
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c => c.GetMember("C")),
},
validator: g =>
{
Expand Down Expand Up @@ -13958,7 +13958,7 @@ class C
}
""",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c=>c.GetMember("C")),
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c => c.GetMember("C")),
},
validator: g =>
{
Expand Down Expand Up @@ -14039,6 +14039,63 @@ .maxstack 1
.Verify();
}

[Fact]
public void Method_Rename_Multiple()
{
using var test = new EditAndContinueTest(options: TestOptions.DebugDll, targetFramework: TargetFramework.NetStandard20)
.AddGeneration(
source: $$"""
class C
{
int M1() { return 0; }
}
""",
validator: g =>
{
g.VerifyTypeDefNames("<Module>", "C");
g.VerifyMethodDefNames("M1", ".ctor");
g.VerifyMemberRefNames(/*CompilationRelaxationsAttribute.*/".ctor", /*RuntimeCompatibilityAttribute.*/".ctor", /*Object.*/".ctor", /*DebuggableAttribute*/".ctor");
});

for (int i = 0; i < 10; i++)
{
test.AddGeneration(
source: @$"
class C
{{
int M2() {{ return {i}; }}
}}",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M1"), newSymbolProvider: c => c.GetMember("C")),
Edit(SemanticEditKind.Insert, symbolProvider: c => c.GetMember("C.M2")),
},
validator: g =>
{
g.VerifyTypeDefNames();
g.VerifyMethodDefNames("M1", "M2");
g.VerifyDeletedMembers("C: {M1}");
})
.AddGeneration(
source: @$"
class C
{{
int M1() {{ return {i}; }}
}}",
edits: new[] {
Edit(SemanticEditKind.Delete, symbolProvider: c => c.GetMember("C.M2"), newSymbolProvider: c => c.GetMember("C")),
Edit(SemanticEditKind.Insert, symbolProvider: c => c.GetMember("C.M1")),
},
validator: g =>
{
g.VerifyTypeDefNames();
g.VerifyMethodDefNames("M1", "M2");
g.VerifyDeletedMembers("C: {M2}");
});
}

test.Verify();
}

[Fact]
public void FileTypes_01()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ internal EmitBaseline GetDelta(Compilation compilation, Guid encId, MetadataSize
// otherwise members from the current compilation have already been merged into the baseline.
var synthesizedMembers = (_previousGeneration.Ordinal == 0) ? module.GetAllSynthesizedMembers() : _previousGeneration.SynthesizedMembers;

Debug.Assert(module.EncSymbolChanges is not null);
var deletedMembers = (_previousGeneration.Ordinal == 0) ? module.EncSymbolChanges.GetAllDeletedMethods() : _previousGeneration.DeletedMembers;
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 didn't do this originally because it is not needed, as it is used when re-adding deleted members, and its impossible to do that in generation 1, but without it the DeletedMembers property of the EmitBaseline doesn't represent the truth for generation 1, which was annoying for tests.


var currentGenerationOrdinal = _previousGeneration.Ordinal + 1;

var addedTypes = _typeDefs.GetAdded();
Expand Down Expand Up @@ -231,7 +234,7 @@ internal EmitBaseline GetDelta(Compilation compilation, Guid encId, MetadataSize
anonymousDelegates: ((IPEDeltaAssemblyBuilder)module).GetAnonymousDelegates(),
anonymousDelegatesWithFixedTypes: ((IPEDeltaAssemblyBuilder)module).GetAnonymousDelegatesWithFixedTypes(),
synthesizedMembers: synthesizedMembers,
deletedMembers: _previousGeneration.DeletedMembers,
deletedMembers: deletedMembers,
addedOrChangedMethods: AddRange(_previousGeneration.AddedOrChangedMethods, addedOrChangedMethodsByIndex),
debugInformationProvider: _previousGeneration.DebugInformationProvider,
localSignatureProvider: _previousGeneration.LocalSignatureProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ internal abstract class SymbolMatcher
}

/// <summary>
/// Merges synthesized members generated during lowering of the current compilation with aggregate synthesized members
/// from all previous source generations (gen >= 1).
/// Merges synthesized or deleted members generated during lowering, or emit, of the current compilation with aggregate
/// synthesized or deleted members from all previous source generations (gen >= 1).
/// </summary>
/// <remarks>
/// Suppose {S -> {A, B, D}, T -> {E, F}} are all synthesized members in previous generations,
Expand All @@ -159,9 +159,10 @@ internal abstract class SymbolMatcher
/// Then the resulting collection shall have the following entries:
/// {S' -> {A', B', C, D}, U -> {G, H}, T -> {E, F}}
/// </remarks>
internal ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> MapSynthesizedMembers(
internal ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> MapSynthesizedOrDeletedMembers(
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> previousMembers,
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> newMembers)
ImmutableDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> newMembers,
bool isDeletedMemberMapping)
{
// Note: we can't just return previous members if there are no new members, since we still need to map the symbols to the new compilation.

Expand All @@ -174,11 +175,8 @@ internal abstract class SymbolMatcher

synthesizedMembersBuilder.AddRange(newMembers);

foreach (var pair in previousMembers)
foreach (var (previousContainer, members) in previousMembers)
{
var previousContainer = pair.Key;
var members = pair.Value;

var mappedContainer = MapDefinitionOrNamespace(previousContainer);
if (mappedContainer == null)
{
Expand Down Expand Up @@ -207,7 +205,11 @@ internal abstract class SymbolMatcher
// If the matcher found a member in the current compilation corresponding to previous memberDef,
// then the member has to be synthesized and produced as a result of a method update
// and thus already contained in newSynthesizedMembers.
Debug.Assert(newSynthesizedMembers.Contains(mappedMember));
// However, because this method is also used to map deleted members, it's possible that a method
// could be renamed in the previous generation, and renamed back in this generation, which would
// mean it could be mapped, but isn't in the newSynthesizedMembers list, so we allow the flag to
// override this behaviour for deleted methods.
Debug.Assert(isDeletedMemberMapping || newSynthesizedMembers.Contains(mappedMember));
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
currentSynthesizedMembers,
currentDeletedMembers)

Dim mappedSynthesizedMembers = matcher.MapSynthesizedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers)
Dim mappedDeletedMembers = matcher.MapSynthesizedMembers(previousGeneration.DeletedMembers, currentDeletedMembers)
Dim mappedSynthesizedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers, isDeletedMemberMapping:=False)
Dim mappedDeletedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.DeletedMembers, currentDeletedMembers, isDeletedMemberMapping:=True)

' TODO can we reuse some data from the previous matcher?
Dim matcherWithAllSynthesizedMembers = New VisualBasicSymbolMatcher(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,43 @@ static void Main(string[] args)
new[] { edits },
new[]
{
DocumentResults(active,
new[] { SemanticEdit(SemanticEditKind.Delete, c => c.GetMember("C.Goo"), deletedSymbolContainerProvider: c => c.GetMember("C")) })
DocumentResults(
active,
diagnostics: new[] { Diagnostic(RudeEditKind.Delete, "class C", DeletedSymbolDisplay(FeaturesResources.method, "Goo(int a)")) })
});
}

[Fact]
public void Method_Rename_Leaf1()
{
var src1 = @"
class C
{
static void Goo(int a)
{
<AS:0>Console.WriteLine(a);</AS:0>
}
}";
var src2 = @"
class C
{
static void Boo(int a)
{
<AS:0>Console.WriteLine(a);</AS:0>
}
}
";

var edits = GetTopEdits(src1, src2);
var active = GetActiveStatements(src1, src2);

EditAndContinueValidation.VerifySemantics(
new[] { edits },
new[]
{
DocumentResults(
active,
diagnostics: new[] {Diagnostic(RudeEditKind.UpdateAroundActiveStatement, "static void Boo(int a)", FeaturesResources.method) })
});
}

Expand Down
Loading