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.

Sign up for GitHub

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

Jump to bottom

[EnC] Store entire spans instead of line deltas #58831

Merged
merged 4 commits into from
Jan 21, 2022
Merged

[EnC] Store entire spans instead of line deltas #58831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a095596
Change LineDelta to NewSpan
davidwengier Jan 13, 2022
296c592
Update tests
davidwengier Jan 13, 2022
6103bfe
Add new test to validate new spans are stored correctly
davidwengier Jan 13, 2022
7430a7c
Rneame Span to OldSpan.
davidwengier Jan 13, 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
67 changes: 67 additions & 0 deletions src/EditorFeatures/Test/EditAndContinue/ActiveStatementsMapTests.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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}";
}
}