Skip to content

Commit

Permalink
Debugger fixes (#441)
Browse files Browse the repository at this point in the history
* Update DebuggerTests.cs

* Bump version

* Update ArrayValue.cs

* Update ValueUtils.cs

* Structuring

* Update install_debugadapter.ps1

* Update SessionCache.cs

* Update Binder_Expression.cs

* Update InlayHint.cs

* Update SequencePointInjector.cs

* Update SequencePointInjector.cs

* Update SequencePointInjector.cs

* Update SyntaxHighlighterTests.cs

* Update ValueUtils.cs
  • Loading branch information
LPeter1997 authored Aug 22, 2024
1 parent 9405f5b commit b848c5f
Show file tree
Hide file tree
Showing 25 changed files with 177 additions and 63 deletions.
2 changes: 1 addition & 1 deletion scripts/install_debugadapter.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

$ErrorActionPreference = "Stop"
Push-Location $PSScriptRoot
dotnet pack ../src/Draco.DebugAdapter --output .
dotnet pack ../src/Draco.DebugAdapter --configuration Debug --output .
if ((dotnet tool list --global) -match "Draco.DebugAdapter") {
dotnet tool uninstall --global Draco.DebugAdapter
}
Expand Down
1 change: 0 additions & 1 deletion src/Draco.Compiler.Tests/Syntax/SyntaxHighlighterTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Collections.Immutable;
using System.Drawing;
using Draco.Compiler.Api.Syntax;
using Draco.Compiler.Tests.Semantics;

Expand Down
21 changes: 14 additions & 7 deletions src/Draco.Compiler/Internal/Binding/Binder_Expression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -628,9 +628,9 @@ private async BindingTask<BoundExpression> BindMemberExpression(MemberExpression
case Symbol when member.IsError:
return new BoundReferenceErrorExpression(syntax, member);
case FunctionSymbol func:
return await this.WrapFunctions(syntax, receiver, [func]);
return await this.WrapFunctions(syntax, receiver, [func], constraints, diagnostics);
case OverloadSymbol overload:
return await this.WrapFunctions(syntax, receiver, overload.Functions);
return await this.WrapFunctions(syntax, receiver, overload.Functions, constraints, diagnostics);
case FieldSymbol field:
return new BoundFieldExpression(syntax, receiver, field);
case PropertySymbol prop:
Expand Down Expand Up @@ -776,9 +776,9 @@ private async BindingTask<BoundExpression> StaticSymbolToExpression(
var getter = GetGetterSymbol(syntax, prop, diagnostics);
return new BoundPropertyGetExpression(syntax, null, getter);
case FunctionSymbol func:
return await this.WrapFunctions(syntax, null, [func]);
return await this.WrapFunctions(syntax, null, [func], constraints, diagnostics);
case OverloadSymbol overload:
return await this.WrapFunctions(syntax, null, overload.Functions);
return await this.WrapFunctions(syntax, null, overload.Functions, constraints, diagnostics);
default:
throw new InvalidOperationException();
}
Expand All @@ -787,7 +787,9 @@ private async BindingTask<BoundExpression> StaticSymbolToExpression(
private BindingTask<BoundExpression> WrapFunctions(
SyntaxNode syntax,
BoundExpression? receiver,
ImmutableArray<FunctionSymbol> functions)
ImmutableArray<FunctionSymbol> functions,
ConstraintSolver constraints,
DiagnosticBag diagnostics)
{
if (IsMethodOfCallExpression(syntax))
{
Expand All @@ -814,8 +816,13 @@ private BindingTask<BoundExpression> WrapFunctions(
}
else
{
// TODO
throw new NotImplementedException();
// TODO: We should construct some constraints to resolve which one
// For now we report an error to not crash tools
diagnostics.Add(Diagnostic.Create(
template: TypeCheckingErrors.IllegalExpression,
location: syntax.Location));
return BindingTask.FromResult<BoundExpression>(
new BoundReferenceErrorExpression(syntax, WellKnownTypes.ErrorType));
}
}
}
Expand Down
105 changes: 101 additions & 4 deletions src/Draco.Compiler/Internal/Lowering/SequencePointInjector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Linq;
using Draco.Compiler.Api.Syntax;
using Draco.Compiler.Internal.BoundTree;
using Draco.Compiler.Internal.Symbols;
using Draco.Compiler.Internal.Symbols.Synthetized;
using static Draco.Compiler.Internal.BoundTree.BoundTreeFactory;

Expand All @@ -12,6 +13,17 @@ namespace Draco.Compiler.Internal.Lowering;
/// </summary>
internal sealed class SequencePointInjector : BoundTreeRewriter
{
/// <summary>
/// Represents a value that was temporarily stored.
/// </summary>
/// <param name="Symbol">The synthetized local symbol.</param>
/// <param name="Reference">The expression referencing the stored temporary.</param>
/// <param name="Assignment">The assignment that stores the temporary.</param>
private readonly record struct TemporaryStorage(
LocalSymbol Symbol,
BoundExpression Reference,
BoundStatement Assignment);

public static BoundNode Inject(BoundNode node)
{
var rewriter = new SequencePointInjector(node);
Expand Down Expand Up @@ -124,9 +136,9 @@ public override BoundNode VisitBlockExpression(BoundBlockExpression node)
statements:
[
SequencePointStatement(
statement: null,
range: openBrace.Range,
emitNop: true),
statement: null,
range: openBrace.Range,
emitNop: true),
LocalDeclaration(blockValue, BlockExpression(
locals: node.Locals,
statements: statements,
Expand Down Expand Up @@ -200,6 +212,74 @@ public override BoundNode VisitWhileExpression(BoundWhileExpression node)
breakLabel: node.BreakLabel);
}

public override BoundNode VisitForExpression(BoundForExpression node)
{
// For loops are a bit more complex, transformation can be found below
//
// for (i in SomeSequence) body;
//
// =>
//
// val storedSequence = SomeSequence; // first sequence point, pointing at 'SomeSequence'
// for (newIterator in storedSequence) {
// ^^^^^^^^^^^^^^ second sequence point for 'in' keyword
// val i = newIterator; // third sequence point, pointing at 'i'
// body;
// }

var syntax = node.Syntax as ForExpressionSyntax;

// Wrap the sequence and store it
var sequence = (BoundExpression)node.Sequence.Accept(this);
var sequenceStorage = StoreTemporary(sequence);
var sequenceAssignment = SequencePointStatement(
statement: sequenceStorage.Assignment,
range: node.Sequence.Syntax?.Range,
emitNop: true);

// Wrap the sequence reference for 'in'
var sequenceIn = SequencePointExpression(
expression: sequenceStorage.Reference,
range: syntax?.InKeyword.Range,
emitNop: true);

// Create a new iterator variable
var newIterator = new SynthetizedLocalSymbol(node.Iterator.Type, false);

// Create the assignment, pointing at the iterator syntax
var iteratorAssignment = SequencePointStatement(
statement: LocalDeclaration(node.Iterator, LocalExpression(newIterator)),
range: syntax?.Iterator.Range,
emitNop: true);

// Wrap the body
var then = (BoundExpression)node.Then.Accept(this);

// Reconstruction
return BlockExpression(
locals: [sequenceStorage.Symbol],
statements:
[
sequenceAssignment,
ExpressionStatement(ForExpression(
iterator: newIterator,
sequence: sequenceIn,
then: BlockExpression(
locals: [node.Iterator],
statements: [
iteratorAssignment,
ExpressionStatement(then),
],
value: BoundUnitExpression.Default),
continueLabel: node.ContinueLabel,
breakLabel: node.BreakLabel,
getEnumeratorMethod: node.GetEnumeratorMethod,
moveNextMethod: node.MoveNextMethod,
currentProperty: node.CurrentProperty)),
],
value: BoundUnitExpression.Default);
}

public override BoundNode VisitReturnExpression(BoundReturnExpression node)
{
var ancestorBlock = this.GetBlockFunctionBodyAncestor();
Expand Down Expand Up @@ -281,9 +361,26 @@ WhileExpressionSyntax @while when ReferenceEquals(syntax, @while.Condition) =>
};
}

// Utility to store an expression to a temporary variable
// NOTE: Almost copypaste from local rewriter, but we always store here
// this is because here the goal is not eliminating double evaluation,
// but to provide a sequence point for a given hidden expression
// We also DO NOT call lowering anywhere
// If the expression passed in is assumed to be injected, caller should cater for that
private static TemporaryStorage StoreTemporary(BoundExpression expr)
{
var symbol = new SynthetizedLocalSymbol(expr.TypeRequired, false);
var symbolRef = LocalExpression(symbol);
var assignment = LocalDeclaration(local: symbol, value: expr);
return new(symbol, symbolRef, assignment);
}

private static bool IsCompoundExpression(BoundExpression expr) => expr switch
{
BoundBlockExpression or BoundWhileExpression or BoundIfExpression => true,
BoundBlockExpression
or BoundWhileExpression
or BoundIfExpression
or BoundForExpression => true,
BoundSequencePointExpression sp => IsCompoundExpression(sp.Expression),
_ => false,
};
Expand Down
1 change: 1 addition & 0 deletions src/Draco.DebugAdapter/DracoDebugAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Draco.Dap.Adapter;
using Draco.Dap.Model;
using Draco.Debugger;
using Draco.Debugger.Events;

namespace Draco.DebugAdapter;

Expand Down
6 changes: 4 additions & 2 deletions src/Draco.DebugAdapter/Translator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Collections.Generic;
using System.IO;
using System.Runtime.CompilerServices;
using Draco.Debugger.Breakpoints;
using Draco.Debugger.RuntimeValues;
using DapModels = Draco.Dap.Model;
using DebuggerApi = Draco.Debugger;

Expand Down Expand Up @@ -44,7 +46,7 @@ public int AllocateId(DapModels.SourceBreakpoint breakpoint)

public IList<DapModels.Variable> GetVariables(int variablesReference)
{
static bool IsCompound(object? value) => value is DebuggerApi.ArrayValue or DebuggerApi.ObjectValue;
static bool IsCompound(object? value) => value is ArrayValue or ObjectValue;

if (!this.valueCache.TryGetValue(variablesReference, out var value)) return Array.Empty<DapModels.Variable>();

Expand Down Expand Up @@ -123,7 +125,7 @@ public DapModels.StackFrame ToDap(DebuggerApi.StackFrame frame)
: thread.Name,
};

public DapModels.Breakpoint ToDap(DebuggerApi.Breakpoint breakpoint, int? id)
public DapModels.Breakpoint ToDap(Breakpoint breakpoint, int? id)
{
var result = new DapModels.Breakpoint()
{
Expand Down
61 changes: 28 additions & 33 deletions src/Draco.Debugger.Tests/DebuggerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ func main() {
i += 1;
}
}
""",
this.output);
""", this.output);
var debugger = session.Debugger;
debugger.Continue();
await debugger.Terminated;
Expand All @@ -39,14 +38,13 @@ func main() {
public async Task SingleBreakpoint() => await Timeout(async () =>
{
var session = await TestDebugSession.DebugAsync("""
import System.Console;
func main() {
WriteLine("A");
WriteLine("B");
WriteLine("C");
}
""",
this.output);
import System.Console;
func main() {
WriteLine("A");
WriteLine("B");
WriteLine("C");
}
""", this.output);
var debugger = session.Debugger;
Assert.True(session.File.TryPlaceBreakpoint(3, out var breakpoint));
debugger.Continue();
Expand All @@ -61,15 +59,14 @@ func main() {
public async Task MultipleBreakpointBreak() => await Timeout(async () =>
{
var session = await TestDebugSession.DebugAsync("""
import System.IO;
func main() {
var i = 0;
while(i < 10) {
i += 1;
}
import System.IO;
func main() {
var i = 0;
while(i < 10) {
i += 1;
}
""",
this.output);
}
""", this.output);

Assert.True(session.File.TryPlaceBreakpoint(5, out var breakpoint));

Expand All @@ -92,15 +89,14 @@ func main() {
public async Task HiddenLocalsDoesNotThrow() => await Timeout(async () =>
{
var session = await TestDebugSession.DebugAsync("""
import System.Console;
func main() {
var i = 1;
WriteLine("\{i.ToString()}a");
WriteLine("\{i.ToString()}b");
WriteLine("\{i.ToString()}c");
}
""",
this.output);
import System.Console;
func main() {
var i = 1;
WriteLine("\{i.ToString()}a");
WriteLine("\{i.ToString()}b");
WriteLine("\{i.ToString()}c");
}
""", this.output);

var debugger = session.Debugger;

Expand All @@ -120,12 +116,11 @@ func main() {
public async Task CanStartProcess() => await Timeout(async () =>
{
var session = await TestDebugSession.DebugAsync("""
import System.Console;
func main() {
WriteLine("Hello, World!");
}
""",
this.output);
import System.Console;
func main() {
WriteLine("Hello, World!");
}
""", this.output);

var debugger = session.Debugger;
var output = null as string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System.Threading.Tasks;
using ClrDebug;

namespace Draco.Debugger;
namespace Draco.Debugger.Breakpoints;

/// <summary>
/// Represents a breakpoint.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using ClrDebug;

namespace Draco.Debugger;
namespace Draco.Debugger.Breakpoints;

internal sealed class EntryPointBreakpoint(
SessionCache sessionCache,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using ClrDebug;

namespace Draco.Debugger;
namespace Draco.Debugger.Breakpoints;

// NOTE: Not sealed, entry-point breakpoint reuses this
internal class MethodBreakpoint : Breakpoint
Expand Down
1 change: 1 addition & 0 deletions src/Draco.Debugger/Debugger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.IO;
using System.Threading.Tasks;
using ClrDebug;
using Draco.Debugger.Breakpoints;
using Draco.Debugger.IO;

namespace Draco.Debugger;
Expand Down
1 change: 1 addition & 0 deletions src/Draco.Debugger/DebuggerEvents.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Draco.Debugger.Events;

namespace Draco.Debugger;

Expand Down
2 changes: 1 addition & 1 deletion src/Draco.Debugger/Draco.Debugger.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="ClrDebug" Version="0.3.2" />
<PackageReference Include="ClrDebug" Version="0.3.3" />
<PackageReference Include="Microsoft.Diagnostics.DbgShim" Version="8.0.505301" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using Draco.Debugger.Breakpoints;

namespace Draco.Debugger;
namespace Draco.Debugger.Events;

/// <summary>
/// The event arguments for the event when a breakpoint is hit.
Expand Down
Loading

0 comments on commit b848c5f

Please sign in to comment.