Skip to content

Commit

Permalink
Improve sequence point placement in primary constructors and records
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmat
tmat committed Jul 18, 2023
1 parent 122b43d commit 68409c1
Show file tree
Hide file tree
Showing 61 changed files with 3,502 additions and 935 deletions.
87 changes: 71 additions & 16 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,65 @@ public override BoundStatement InstrumentExpressionStatement(BoundExpressionStat

if (original.IsConstructorInitializer())
{
switch (original.Syntax.Kind())
switch (original.Syntax)
{
case SyntaxKind.ConstructorDeclaration:
// This is an implicit constructor initializer.
var decl = (ConstructorDeclarationSyntax)original.Syntax;
return new BoundSequencePointWithSpan(decl, rewritten, CreateSpanForConstructorInitializer(decl));
case SyntaxKind.BaseConstructorInitializer:
case SyntaxKind.ThisConstructorInitializer:
var init = (ConstructorInitializerSyntax)original.Syntax;
Debug.Assert(init.Parent is object);
return new BoundSequencePointWithSpan(init, rewritten, CreateSpanForConstructorInitializer((ConstructorDeclarationSyntax)init.Parent));
case ConstructorDeclarationSyntax ctorDecl:
// Implicit constructor initializer:
return new BoundSequencePointWithSpan(ctorDecl, rewritten, createSpanForConstructorInitializer(ctorDecl));

case ConstructorInitializerSyntax ctorInit:
// Explicit constructor initializer:
Debug.Assert(ctorInit.Parent is ConstructorDeclarationSyntax);
return new BoundSequencePointWithSpan(ctorInit, rewritten, createSpanForConstructorInitializer((ConstructorDeclarationSyntax)ctorInit.Parent));

case TypeDeclarationSyntax typeDecl:
// Primary constructor with implicit base initializer:
// class [|C<T>(int X, int Y)|] : B;
Debug.Assert(typeDecl.ParameterList != null);
Debug.Assert(typeDecl.BaseList?.Types.Any(t => t is PrimaryConstructorBaseTypeSyntax { ArgumentList: not null }) != true);

return new BoundSequencePointWithSpan(typeDecl, rewritten, TextSpan.FromBounds(typeDecl.Identifier.SpanStart, typeDecl.ParameterList.Span.End));

case PrimaryConstructorBaseTypeSyntax baseInit:
// Explicit base initializer:
// class C<T>(int X, int Y) : [|B(...)|];
return new BoundSequencePointWithSpan(baseInit, rewritten, baseInit.Span);

default:
throw ExceptionUtilities.UnexpectedValue(original.Syntax.Kind());
}
}
else if (original.Syntax is ParameterSyntax parameterSyntax)
{
// Record property setter sequence point.
return new BoundSequencePointWithSpan(parameterSyntax, rewritten, CreateSpan(parameterSyntax));
}

return AddSequencePoint(rewritten);

static TextSpan createSpanForConstructorInitializer(ConstructorDeclarationSyntax constructorSyntax)
{
if (constructorSyntax.Initializer != null)
{
// [SomeAttribute] public MyCtorName(params int[] values): [|base()|] { ... }
var start = constructorSyntax.Initializer.ThisOrBaseKeyword.SpanStart;
var end = constructorSyntax.Initializer.ArgumentList.CloseParenToken.Span.End;
return TextSpan.FromBounds(start, end);
}

if (constructorSyntax.Modifiers.Any(SyntaxKind.StaticKeyword))
{
Debug.Assert(constructorSyntax.Body != null);

// [SomeAttribute] static MyCtorName(...) [|{|] ... }
var start = constructorSyntax.Body.OpenBraceToken.SpanStart;
var end = constructorSyntax.Body.OpenBraceToken.Span.End;
return TextSpan.FromBounds(start, end);
}

// [SomeAttribute] [|public MyCtorName(params int[] values)|] { ... }
return CreateSpan(constructorSyntax.Modifiers, constructorSyntax.Identifier, constructorSyntax.ParameterList.CloseParenToken);
}
}

public override BoundStatement InstrumentFieldOrPropertyInitializer(BoundStatement original, BoundStatement rewritten)
Expand All @@ -94,7 +138,8 @@ private static BoundStatement InstrumentFieldOrPropertyInitializer(BoundStatemen
if (syntax.IsKind(SyntaxKind.Parameter))
{
// This is an initialization of a generated property based on record parameter.
return AddSequencePoint(rewritten);
// Do not add sequence point - the initialization is trivial and there is no value stepping over every backing field assignment.
return rewritten;
}

Debug.Assert(syntax is { Parent: { Parent: { } } });
Expand Down Expand Up @@ -163,14 +208,18 @@ public override void InstrumentBlock(BoundBlock original, LocalRewriter rewriter
}
else if (original == rewriter.CurrentMethodBody)
{
if (previousPrologue != null)
// Add hidden sequence point at the start of
// 1) a prologue added by instrumentation.
// 2) a synthesized main entry point for top-level code.
// 3) synthesized body of primary and copy constructors of a record type that has at least one parameter.
// This hidden sequence point covers assignments of parameters/original fields to the current instance backing fields.
// These assignments do not have their own sequence points.
if (previousPrologue != null ||
rewriter.Factory.TopLevelMethod is SynthesizedSimpleProgramEntryPointSymbol ||
original.Syntax is RecordDeclarationSyntax { ParameterList.Parameters.Count: > 0 })
{
prologue = BoundSequencePoint.CreateHidden(previousPrologue);
}
else if (rewriter.Factory.TopLevelMethod is SynthesizedSimpleProgramEntryPointSymbol)
{
prologue = BoundSequencePoint.CreateHidden();
}

if (previousEpilogue != null)
{
Expand Down Expand Up @@ -364,6 +413,12 @@ public override BoundStatement InstrumentReturnStatement(BoundReturnStatement or
return new BoundSequencePointWithSpan(original.Syntax, rewritten, ((BlockSyntax)original.Syntax).CloseBraceToken.Span);
}

if (original.Syntax is ParameterSyntax parameterSyntax)
{
// Record property getter sequence point
return new BoundSequencePointWithSpan(parameterSyntax, rewritten, CreateSpan(parameterSyntax));
}

return new BoundSequencePoint(original.Syntax, rewritten);
}

Expand Down
27 changes: 4 additions & 23 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector_SequencePoints.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,29 +45,10 @@ internal static BoundStatement AddSequencePoint(UsingStatementSyntax usingSyntax
return new BoundSequencePointWithSpan(usingSyntax, rewrittenStatement, span);
}

private static TextSpan CreateSpanForConstructorInitializer(ConstructorDeclarationSyntax constructorSyntax)
{
if (constructorSyntax.Initializer != null)
{
// [SomeAttribute] public MyCtorName(params int[] values): [|base()|] { ... }
var start = constructorSyntax.Initializer.ThisOrBaseKeyword.SpanStart;
var end = constructorSyntax.Initializer.ArgumentList.CloseParenToken.Span.End;
return TextSpan.FromBounds(start, end);
}

if (constructorSyntax.Modifiers.Any(SyntaxKind.StaticKeyword))
{
Debug.Assert(constructorSyntax.Body != null);

// [SomeAttribute] static MyCtorName(...) [|{|] ... }
var start = constructorSyntax.Body.OpenBraceToken.SpanStart;
var end = constructorSyntax.Body.OpenBraceToken.Span.End;
return TextSpan.FromBounds(start, end);
}

// [SomeAttribute] [|public MyCtorName(params int[] values)|] { ... }
return CreateSpan(constructorSyntax.Modifiers, constructorSyntax.Identifier, constructorSyntax.ParameterList.CloseParenToken);
}
private static TextSpan CreateSpan(ParameterSyntax parameter)
// exclude attributes and default value:
// [A] [|in T p|] = default
=> CreateSpan(parameter.Modifiers, parameter.Type, parameter.Identifier);

private static TextSpan CreateSpan(SyntaxTokenList startOpt, SyntaxNodeOrToken startFallbackOpt, SyntaxNodeOrToken endOpt)
{
Expand Down
18 changes: 18 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
Expand Down Expand Up @@ -53,6 +55,22 @@ internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F,
statements.Add(F.Assignment(F.Field(F.This(), field), F.Field(param, field)));
}
}

// Add a sequence point at the end of the copy-constructor, so that a breakpoint placed on the record constructor
// can be hit whenever a new instance of the record is created (either via a primary constructor or vie a copy constructor).
//
// If the record doesn't have base initializer the span overlaps the span of the sequence point generated for primary
// constructor implicit base initializer:
//
// record [|C<T>(int P, int Q)|] // implicit base constructor initializer span
// record C<T>(int P, int Q) : [|B(...)|] // explicit base constructor initializer span
// record [|C<T>|](int P, int Q) // copy-constructor span
//
// The copy-constructor span does not include the parameter list since the parameters are not in scope.
var recordDeclaration = (RecordDeclarationSyntax)F.Syntax;
statements.Add(new BoundSequencePointWithSpan(recordDeclaration, statementOpt: null,
(recordDeclaration.TypeParameterList == null) ? recordDeclaration.Identifier.Span :
TextSpan.FromBounds(recordDeclaration.Identifier.Span.Start, recordDeclaration.TypeParameterList.Span.End)));
}

internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<SynthesizedAttributeData> attributes)
Expand Down
36 changes: 30 additions & 6 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1912,7 +1912,7 @@ public static void M(int a)
}

[Fact]
public void Record_1()
public void LiftedPrimaryParameter_Record()
{
var source = WithWindowsLineBreaks(@"
using System;
Expand Down Expand Up @@ -1947,9 +1947,9 @@ record D(int X)
</customDebugInfo>
<sequencePoints>
<entry offset=""0x0"" hidden=""true"" document=""1"" />
<entry offset=""0xd"" startLine=""3"" startColumn=""10"" endLine=""3"" endColumn=""15"" document=""1"" />
<entry offset=""0xd"" hidden=""true"" document=""1"" />
<entry offset=""0x19"" startLine=""5"" startColumn=""34"" endLine=""5"" endColumn=""74"" document=""1"" />
<entry offset=""0x31"" startLine=""3"" startColumn=""1"" endLine=""6"" endColumn=""2"" document=""1"" />
<entry offset=""0x31"" startLine=""3"" startColumn=""8"" endLine=""3"" endColumn=""16"" document=""1"" />
</sequencePoints>
<scope startOffset=""0x0"" endOffset=""0x39"">
<namespace name=""System"" />
Expand All @@ -1976,6 +1976,15 @@ record D(int X)
<entry offset=""0x0"" startLine=""5"" startColumn=""25"" endLine=""5"" endColumn=""29"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""D"" name="".ctor"" parameterNames=""original"">
<customDebugInfo>
<forward declaringType=""D"" methodName="".ctor"" parameterNames=""X"" />
</customDebugInfo>
<sequencePoints>
<entry offset=""0x0"" hidden=""true"" document=""1"" />
<entry offset=""0x1f"" startLine=""3"" startColumn=""8"" endLine=""3"" endColumn=""9"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""D+&lt;&gt;c__DisplayClass0_0"" name=""&lt;.ctor&gt;b__0"" parameterNames=""a"">
<customDebugInfo>
<forward declaringType=""D"" methodName="".ctor"" parameterNames=""X"" />
Expand All @@ -1989,7 +1998,7 @@ record D(int X)
}

[Fact]
public void Record_2()
public void PrimaryBaseInitializer()
{
var source = WithWindowsLineBreaks(@"
using System;
Expand Down Expand Up @@ -2028,8 +2037,8 @@ static int F(int x, out int p)
</using>
</customDebugInfo>
<sequencePoints>
<entry offset=""0x0"" startLine=""3"" startColumn=""10"" endLine=""3"" endColumn=""15"" document=""1"" />
<entry offset=""0x7"" startLine=""3"" startColumn=""1"" endLine=""9"" endColumn=""2"" document=""1"" />
<entry offset=""0x0"" hidden=""true"" document=""1"" />
<entry offset=""0x7"" startLine=""3"" startColumn=""8"" endLine=""3"" endColumn=""16"" document=""1"" />
</sequencePoints>
<scope startOffset=""0x0"" endOffset=""0xf"">
<namespace name=""System"" />
Expand All @@ -2055,6 +2064,15 @@ static int F(int x, out int p)
<entry offset=""0x9"" startLine=""8"" startColumn=""5"" endLine=""8"" endColumn=""6"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""C"" name="".ctor"" parameterNames=""original"">
<customDebugInfo>
<forward declaringType=""C"" methodName="".ctor"" parameterNames=""X"" />
</customDebugInfo>
<sequencePoints>
<entry offset=""0x0"" hidden=""true"" document=""1"" />
<entry offset=""0x13"" startLine=""3"" startColumn=""8"" endLine=""3"" endColumn=""9"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""D"" name="".ctor"" parameterNames=""X"">
<customDebugInfo>
<forward declaringType=""C"" methodName="".ctor"" parameterNames=""X"" />
Expand Down Expand Up @@ -2089,6 +2107,12 @@ static int F(int x, out int p)
<entry offset=""0xa"" startLine=""17"" startColumn=""5"" endLine=""17"" endColumn=""6"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""D"" name="".ctor"" parameterNames=""original"">
<sequencePoints>
<entry offset=""0x0"" hidden=""true"" document=""1"" />
<entry offset=""0x8"" startLine=""11"" startColumn=""8"" endLine=""11"" endColumn=""9"" document=""1"" />
</sequencePoints>
</method>
<method containingType=""D+&lt;&gt;c__DisplayClass0_0"" name=""&lt;.ctor&gt;b__0"">
<customDebugInfo>
<forward declaringType=""C"" methodName="".ctor"" parameterNames=""X"" />
Expand Down
Loading

0 comments on commit 68409c1

Please sign in to comment.