Skip to content

Commit

Permalink
[EnC] Store entire spans instead of line deltas (#58831)
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidwengier
davidwengier authored Jan 21, 2022
1 parent 68818a0 commit c05463a
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 55 deletions.
67 changes: 67 additions & 0 deletions src/EditorFeatures/Test/EditAndContinue/ActiveStatementsMapTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,5 +125,72 @@ ManagedActiveStatementDebugInfo CreateInfo(int startLine, int startColumn, int e
"[134..138) -> (6,0)-(6,4) #1"
}, oldSpans.Select(s => $"{s.UnmappedSpan} -> {s.Statement.Span} #{s.Statement.Ordinal}"));
}

[Fact]
public void ExpandMultiLineSpan()
{
using var workspace = new TestWorkspace(composition: FeaturesTestCompositions.Features);

var source = @"
using System;
class C
{
void F()
{
G(() =>
{
Console.WriteLine(1);
});
}
static void F(Action a)
{
a();
}
}";

var solution = workspace.CurrentSolution
.AddProject("proj", "proj", LanguageNames.CSharp)
.AddDocument("doc", SourceText.From(source, Encoding.UTF8), filePath: "a.cs").Project.Solution;

var project = solution.Projects.Single();
var document = project.Documents.Single();
var analyzer = project.LanguageServices.GetRequiredService<IEditAndContinueAnalyzer>();

var documentPathMap = new Dictionary<string, ImmutableArray<ActiveStatement>>();

var moduleId = Guid.NewGuid();
var token = 0x06000001;
ManagedActiveStatementDebugInfo CreateInfo(int startLine, int startColumn, int endLine, int endColumn)
=> new(new(new(moduleId, token++, version: 1), ilOffset: 0), "a.cs", new SourceSpan(startLine, startColumn, endLine, endColumn), ActiveStatementFlags.NonLeafFrame);

var debugInfos = ImmutableArray.Create(
CreateInfo(9, 0, 9, 34), // Console.WriteLine(1)
CreateInfo(7, 0, 10, 12), // Lambda
CreateInfo(15, 0, 15, 13) // a()
);

var remapping = ImmutableDictionary.CreateBuilder<ManagedMethodId, ImmutableArray<NonRemappableRegion>>();

CreateRegion(0, Span(9, 0, 10, 34), Span(9, 0, 9, 34)); // Current active statement doesn't move
CreateRegion(1, Span(7, 0, 10, 12), Span(7, 0, 15, 12)); // Insert 5 lines inside the lambda
CreateRegion(2, Span(15, 0, 15, 13), Span(20, 0, 20, 13)); // a() call moves down 5 lines

var map = ActiveStatementsMap.Create(debugInfos, remapping.ToImmutable());

AssertEx.Equal(new[]
{
"(7,0)-(15,12)",
"(9,0)-(9,34)",
"(20,0)-(20,13)"
}, map.DocumentPathMap["a.cs"].OrderBy(s => s.Span.Start.Line).Select(s => $"{s.Span}"));

void CreateRegion(int ordinal, SourceFileSpan oldSpan, SourceFileSpan newSpan)
=> remapping.Add(debugInfos[ordinal].ActiveInstruction.Method, ImmutableArray.Create(new NonRemappableRegion(oldSpan, newSpan, isExceptionRegion: false)));

SourceFileSpan Span(int startLine, int startColumn, int endLine, int endColumn)
=> new("a.cs", new(new(startLine, startColumn), new(endLine, endColumn)));
}
}
}
14 changes: 7 additions & 7 deletions src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3680,8 +3680,8 @@ static void F()

AssertEx.Equal(new[]
{
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) δ=0",
$"0x06000003 v1 | AS {document.FilePath}: (9,14)-(9,18) δ=1",
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) => (4,41)-(4,42)",
$"0x06000003 v1 | AS {document.FilePath}: (9,14)-(9,18) => (10,14)-(10,18)",
}, InspectNonRemappableRegions(debuggingSession.EditSession.NonRemappableRegions));

ExitBreakState(debuggingSession);
Expand All @@ -3701,8 +3701,8 @@ static void F()
// the regions remain unchanged
AssertEx.Equal(new[]
{
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) δ=0",
$"0x06000003 v1 | AS {document.FilePath}: (9,14)-(9,18) δ=1",
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) => (4,41)-(4,42)",
$"0x06000003 v1 | AS {document.FilePath}: (9,14)-(9,18) => (10,14)-(10,18)",
}, InspectNonRemappableRegions(debuggingSession.EditSession.NonRemappableRegions));

// EnC update F v3 -> v4
Expand Down Expand Up @@ -3737,7 +3737,7 @@ static void F()
// Stale active statement region is gone.
AssertEx.Equal(new[]
{
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) δ=0",
$"0x06000002 v1 | AS {document.FilePath}: (4,41)-(4,42) => (4,41)-(4,42)",
}, InspectNonRemappableRegions(debuggingSession.EditSession.NonRemappableRegions));

ExitBreakState(debuggingSession);
Expand Down Expand Up @@ -3862,8 +3862,8 @@ static void F()

AssertEx.Equal(new[]
{
$"0x06000002 v1 | AS {document.FilePath}: (3,41)-(3,42) δ=0",
$"0x06000003 v1 | AS {document.FilePath}: (7,14)-(7,18) δ=2",
$"0x06000002 v1 | AS {document.FilePath}: (3,41)-(3,42) => (3,41)-(3,42)",
$"0x06000003 v1 | AS {document.FilePath}: (7,14)-(7,18) => (9,14)-(9,18)",
}, InspectNonRemappableRegions(debuggingSession.EditSession.NonRemappableRegions));

ExitBreakState(debuggingSession);
Expand Down
50 changes: 25 additions & 25 deletions src/EditorFeatures/Test/EditAndContinue/EditSessionActiveStatementsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,11 @@ static void Main()

AssertEx.Equal(new[]
{
$"0x06000004 v1 | AS {document2.FilePath}: (8,20)-(8,25) δ=1",
$"0x06000004 v1 | ER {document2.FilePath}: (14,8)-(16,9) δ=1",
$"0x06000004 v1 | ER {document2.FilePath}: (10,10)-(12,11) δ=1",
$"0x06000003 v1 | AS {document2.FilePath}: (21,14)-(21,24) δ=0",
$"0x06000005 v1 | AS {document2.FilePath}: (26,20)-(26,25) δ=0"
$"0x06000004 v1 | AS {document2.FilePath}: (8,20)-(8,25) => (9,20)-(9,25)",
$"0x06000004 v1 | ER {document2.FilePath}: (14,8)-(16,9) => (15,8)-(17,9)",
$"0x06000004 v1 | ER {document2.FilePath}: (10,10)-(12,11) => (11,10)-(13,11)",
$"0x06000003 v1 | AS {document2.FilePath}: (21,14)-(21,24) => (21,14)-(21,24)",
$"0x06000005 v1 | AS {document2.FilePath}: (26,20)-(26,25) => (26,20)-(26,25)"
}, nonRemappableRegions.Select(r => $"{r.Method.GetDebuggerDisplay()} | {r.Region.GetDebuggerDisplay()}"));

AssertEx.Equal(new[]
Expand Down Expand Up @@ -388,10 +388,10 @@ static void F2()

AssertEx.Equal(new[]
{
$"0x06000001 v1 | AS {document.FilePath}: (6,18)-(6,23) δ=0",
$"0x06000001 v1 | ER {document.FilePath}: (8,8)-(12,9) δ=0",
$"0x06000002 v1 | AS {document.FilePath}: (18,14)-(18,36) δ=0",
}, nonRemappableRegions.OrderBy(r => r.Region.Span.Span.Start.Line).Select(r => $"{r.Method.GetDebuggerDisplay()} | {r.Region.GetDebuggerDisplay()}"));
$"0x06000001 v1 | AS {document.FilePath}: (6,18)-(6,23) => (6,18)-(6,23)",
$"0x06000001 v1 | ER {document.FilePath}: (8,8)-(12,9) => (8,8)-(12,9)",
$"0x06000002 v1 | AS {document.FilePath}: (18,14)-(18,36) => (18,14)-(18,36)",
}, nonRemappableRegions.OrderBy(r => r.Region.OldSpan.Span.Start.Line).Select(r => $"{r.Method.GetDebuggerDisplay()} | {r.Region.GetDebuggerDisplay()}"));

AssertEx.Equal(new[]
{
Expand Down Expand Up @@ -503,16 +503,16 @@ static void F4()
{
{ new ManagedMethodId(module1, 0x06000003, 1), ImmutableArray.Create(
// move AS:2 one line up:
new NonRemappableRegion(spanPreRemap2, lineDelta: -1, isExceptionRegion: false),
new NonRemappableRegion(spanPreRemap2, spanPreRemap2.AddLineDelta(-1), isExceptionRegion: false),
// move ER:2.0 and ER:2.1 two lines down:
new NonRemappableRegion(erPreRemap20, lineDelta: +2, isExceptionRegion: true),
new NonRemappableRegion(erPreRemap21, lineDelta: +2, isExceptionRegion: true)) },
new NonRemappableRegion(erPreRemap20, erPreRemap20.AddLineDelta(+2), isExceptionRegion: true),
new NonRemappableRegion(erPreRemap21, erPreRemap21.AddLineDelta(+2), isExceptionRegion: true)) },
{ new ManagedMethodId(module1, 0x06000004, 1), ImmutableArray.Create(
// move AS:3 one line down:
new NonRemappableRegion(spanPreRemap3, lineDelta: +1, isExceptionRegion: false),
new NonRemappableRegion(spanPreRemap3, spanPreRemap3.AddLineDelta(+1), isExceptionRegion: false),
// move ER:3.0 and ER:3.1 one line down:
new NonRemappableRegion(erPreRemap30, lineDelta: +1, isExceptionRegion: true),
new NonRemappableRegion(erPreRemap31, lineDelta: +1, isExceptionRegion: true)) }
new NonRemappableRegion(erPreRemap30, erPreRemap30.AddLineDelta(+1), isExceptionRegion: true),
new NonRemappableRegion(erPreRemap31, erPreRemap31.AddLineDelta(+1), isExceptionRegion: true)) }
}.ToImmutableDictionary();

using var workspace = new TestWorkspace(composition: s_composition);
Expand Down Expand Up @@ -582,16 +582,16 @@ static void F4()
// Note: Since no method have been remapped yet all the following spans are in their pre-remap locations:
AssertEx.Equal(new[]
{
$"0x06000001 v2 | AS {document.FilePath}: (6,18)-(6,22) δ=0",
$"0x06000002 v2 | ER {document.FilePath}: (18,16)-(21,9) δ=-1",
$"0x06000002 v2 | AS {document.FilePath}: (20,18)-(20,22) δ=-1",
$"0x06000003 v1 | AS {document.FilePath}: (30,22)-(30,26) δ=-1", // AS:2 moved -1 in first edit, 0 in second
$"0x06000003 v1 | ER {document.FilePath}: (32,20)-(34,13) δ=2", // ER:2.0 moved +2 in first edit, 0 in second
$"0x06000003 v1 | ER {document.FilePath}: (36,16)-(38,9) δ=2", // ER:2.0 moved +2 in first edit, 0 in second
$"0x06000004 v1 | ER {document.FilePath}: (50,20)-(53,13) δ=3", // ER:3.0 moved +1 in first edit, +2 in second
$"0x06000004 v1 | AS {document.FilePath}: (52,22)-(52,26) δ=3", // AS:3 moved +1 in first edit, +2 in second
$"0x06000004 v1 | ER {document.FilePath}: (55,16)-(57,9) δ=3", // ER:3.1 moved +1 in first edit, +2 in second
}, nonRemappableRegions.OrderBy(r => r.Region.Span.Span.Start.Line).Select(r => $"{r.Method.GetDebuggerDisplay()} | {r.Region.GetDebuggerDisplay()}"));
$"0x06000001 v2 | AS {document.FilePath}: (6,18)-(6,22) => (6,18)-(6,22)",
$"0x06000002 v2 | ER {document.FilePath}: (18,16)-(21,9) => (17,16)-(20,9)",
$"0x06000002 v2 | AS {document.FilePath}: (20,18)-(20,22) => (19,18)-(19,22)",
$"0x06000003 v1 | AS {document.FilePath}: (30,22)-(30,26) => (29,22)-(29,26)", // AS:2 moved -1 in first edit, 0 in second
$"0x06000003 v1 | ER {document.FilePath}: (32,20)-(34,13) => (34,20)-(36,13)", // ER:2.0 moved +2 in first edit, 0 in second
$"0x06000003 v1 | ER {document.FilePath}: (36,16)-(38,9) => (38,16)-(40,9)", // ER:2.0 moved +2 in first edit, 0 in second
$"0x06000004 v1 | ER {document.FilePath}: (50,20)-(53,13) => (53,20)-(56,13)", // ER:3.0 moved +1 in first edit, +2 in second
$"0x06000004 v1 | AS {document.FilePath}: (52,22)-(52,26) => (55,22)-(55,26)", // AS:3 moved +1 in first edit, +2 in second
$"0x06000004 v1 | ER {document.FilePath}: (55,16)-(57,9) => (58,16)-(60,9)", // ER:3.1 moved +1 in first edit, +2 in second
}, nonRemappableRegions.OrderBy(r => r.Region.OldSpan.Span.Start.Line).Select(r => $"{r.Method.GetDebuggerDisplay()} | {r.Region.GetDebuggerDisplay()}"));

AssertEx.Equal(new[]
{
Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Portable/EditAndContinue/ActiveStatementsMap.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ private static bool TryGetUpToDateSpan(ManagedActiveStatementDebugInfo activeSta
{
foreach (var region in regionsInMethod)
{
if (region.Span.Span.Contains(activeSpan) && activeStatementInfo.DocumentName == region.Span.Path)
if (region.OldSpan.Span.Contains(activeSpan) && activeStatementInfo.DocumentName == region.OldSpan.Path)
{
newSpan = activeSpan.AddLineDelta(region.LineDelta);
newSpan = region.NewSpan.Span;
return true;
}
}
Expand Down
17 changes: 8 additions & 9 deletions src/Features/Core/Portable/EditAndContinue/EditSession.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1011,8 +1011,7 @@ void AddNonRemappableRegion(SourceFileSpan oldSpan, SourceFileSpan newSpan, bool
// when break state was entered and now being updated (regardless of whether the active span changed or not).
if (isMethodUpdated)
{
var lineDelta = oldSpan.Span.GetLineDelta(newSpan: newSpan.Span);
nonRemappableRegionsBuilder.Add((methodId, new NonRemappableRegion(oldSpan, lineDelta, isExceptionRegion)));
nonRemappableRegionsBuilder.Add((methodId, new NonRemappableRegion(oldSpan, newSpan, isExceptionRegion)));
}
else if (!isExceptionRegion)
{
Expand All @@ -1024,7 +1023,7 @@ void AddNonRemappableRegion(SourceFileSpan oldSpan, SourceFileSpan newSpan, bool
// That said, we still add a non-remappable region for this active statement, so that we know in future sessions
// that this active statement existed and its span has not changed. We don't report these regions to the debugger,
// but we use them to map active statement spans to the baseline snapshots of following edit sessions.
nonRemappableRegionsBuilder.Add((methodId, new NonRemappableRegion(oldSpan, lineDelta: 0, isExceptionRegion: false)));
nonRemappableRegionsBuilder.Add((methodId, new NonRemappableRegion(oldSpan, oldSpan, isExceptionRegion: false)));
}
}
else if (oldSpan.Span != newSpan.Span)
Expand Down Expand Up @@ -1080,17 +1079,17 @@ void AddNonRemappableRegion(SourceFileSpan oldSpan, SourceFileSpan newSpan, bool
foreach (var region in regionsInMethod)
{
// We have calculated changes against a base snapshot (last break state):
var baseSpan = region.Span.AddLineDelta(region.LineDelta);
var baseSpan = region.NewSpan;

NonRemappableRegion newRegion;
if (changedNonRemappableSpans.TryGetValue((methodInstance.Method, baseSpan), out var newSpan))
{
// all spans must be of the same size:
Debug.Assert(newSpan.Span.End.Line - newSpan.Span.Start.Line == baseSpan.Span.End.Line - baseSpan.Span.Start.Line);
Debug.Assert(region.Span.Span.End.Line - region.Span.Span.Start.Line == baseSpan.Span.End.Line - baseSpan.Span.Start.Line);
Debug.Assert(newSpan.Path == region.Span.Path);
Debug.Assert(region.OldSpan.Span.End.Line - region.OldSpan.Span.Start.Line == baseSpan.Span.End.Line - baseSpan.Span.Start.Line);
Debug.Assert(newSpan.Path == region.OldSpan.Path);

newRegion = region.WithLineDelta(region.Span.Span.GetLineDelta(newSpan: newSpan.Span));
newRegion = region.WithNewSpan(newSpan);
}
else
{
Expand Down Expand Up @@ -1123,8 +1122,8 @@ void AddNonRemappableRegion(SourceFileSpan oldSpan, SourceFileSpan newSpan, bool
r => r.Region.IsExceptionRegion,
r => new ManagedExceptionRegionUpdate(
r.Method,
-r.Region.LineDelta,
r.Region.Span.AddLineDelta(r.Region.LineDelta).Span.ToSourceSpan()));
-r.Region.OldSpan.Span.GetLineDelta(r.Region.NewSpan.Span),
r.Region.NewSpan.Span.ToSourceSpan()));
}
}
}
24 changes: 12 additions & 12 deletions src/Features/Core/Portable/EditAndContinue/NonRemappableRegion.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,46 +14,46 @@ namespace Microsoft.CodeAnalysis.EditAndContinue
/// <summary>
/// Pre-remap PDB span.
/// </summary>
public readonly SourceFileSpan Span;
public readonly SourceFileSpan OldSpan;

/// <summary>
/// Difference between new span and pre-remap span (new = old + delta).
/// New PDB span.
/// </summary>
public readonly int LineDelta;
public readonly SourceFileSpan NewSpan;

/// <summary>
/// True if the region represents an exception region, false if it represents an active statement.
/// </summary>
public readonly bool IsExceptionRegion;

public NonRemappableRegion(SourceFileSpan span, int lineDelta, bool isExceptionRegion)
public NonRemappableRegion(SourceFileSpan oldSpan, SourceFileSpan newSpan, bool isExceptionRegion)
{
Span = span;
LineDelta = lineDelta;
OldSpan = oldSpan;
NewSpan = newSpan;
IsExceptionRegion = isExceptionRegion;
}

public override bool Equals(object? obj)
=> obj is NonRemappableRegion region && Equals(region);

public bool Equals(NonRemappableRegion other)
=> Span.Equals(other.Span) &&
LineDelta == other.LineDelta &&
=> OldSpan.Equals(other.OldSpan) &&
NewSpan.Equals(other.NewSpan) &&
IsExceptionRegion == other.IsExceptionRegion;

public override int GetHashCode()
=> Hash.Combine(Span.GetHashCode(), Hash.Combine(IsExceptionRegion, LineDelta));
=> Hash.Combine(OldSpan.GetHashCode(), Hash.Combine(IsExceptionRegion, NewSpan.GetHashCode()));

public static bool operator ==(NonRemappableRegion left, NonRemappableRegion right)
=> left.Equals(right);

public static bool operator !=(NonRemappableRegion left, NonRemappableRegion right)
=> !(left == right);

public NonRemappableRegion WithLineDelta(int value)
=> new(Span, value, IsExceptionRegion);
public NonRemappableRegion WithNewSpan(SourceFileSpan newSpan)
=> new(OldSpan, newSpan, IsExceptionRegion);

internal string GetDebuggerDisplay()
=> $"{(IsExceptionRegion ? "ER" : "AS")} {Span} δ={LineDelta}";
=> $"{(IsExceptionRegion ? "ER" : "AS")} {OldSpan} => {NewSpan.Span}";
}
}

0 comments on commit c05463a

Please sign in to comment.