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

Add internal instrumentation API for switch expression arm's expression #43430

Merged
11 commits merged into from
Apr 27, 2020
Merged

Add internal instrumentation API for switch expression arm's expression #43430

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5469157
Add internal instrumentation API for switch expression arm's expression
Apr 16, 2020
5f72ae8
Rename GenerateSequencePoints to GenerateInstrumentation
Apr 20, 2020
a31caa6
Merge branch 'features/patterns3' of https://github.com/dotnet/roslyn…
Apr 21, 2020
305734c
Do not generate switch statement instrumentation for synthetic switch…
Apr 21, 2020
8d9ba66
Add mention of new pattern forms in test plan.
Apr 21, 2020
4d380c5
Merge branch 'features/patterns3' of https://github.com/dotnet/roslyn…
Apr 22, 2020
b21dd7d
Merge branch 'features/patterns3' of https://github.com/dotnet/roslyn…
Apr 22, 2020
ddfa6a8
Merge branch 'patterns3-42810' of https://github.com/gafter/roslyn in…
Apr 22, 2020
db9d300
Merge branch 'features/patterns3' of https://github.com/dotnet/roslyn…
Apr 23, 2020
cc5b09b
Resolve an initialization order bug in PatternLocalRewriter constructor
Apr 23, 2020
9b5cbb0
Merge branch 'features/patterns3' of https://github.com/dotnet/roslyn…
Apr 24, 2020
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
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/CompoundInstrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ public override BoundExpression InstrumentSwitchStatementExpression(BoundStateme
return Previous.InstrumentSwitchStatementExpression(original, rewrittenExpression, factory);
}

public override BoundExpression InstrumentSwitchExpressionArmExpression(BoundExpression original, BoundExpression rewrittenExpression, SyntheticBoundNodeFactory factory)
{
return Previous.InstrumentSwitchExpressionArmExpression(original, rewrittenExpression, factory);
}

public override BoundStatement InstrumentSwitchBindCasePatternVariables(BoundStatement bindings)
{
return Previous.InstrumentSwitchBindCasePatternVariables(bindings);
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,11 @@ public override BoundExpression InstrumentSwitchStatementExpression(BoundStateme
return AddConditionSequencePoint(base.InstrumentSwitchStatementExpression(original, rewrittenExpression, factory), original.Syntax, factory);
}

public override BoundExpression InstrumentSwitchExpressionArmExpression(BoundExpression original, BoundExpression rewrittenExpression, SyntheticBoundNodeFactory factory)
{
return new BoundSequencePointExpression(original.Syntax, base.InstrumentSwitchExpressionArmExpression(original, rewrittenExpression, factory), rewrittenExpression.Type);
}

public override BoundStatement InstrumentSwitchBindCasePatternVariables(BoundStatement bindings)
{
// Mark the code that binds pattern variables to their values as hidden.
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/Instrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,15 @@ public virtual BoundExpression InstrumentSwitchStatementExpression(BoundStatemen
return rewrittenExpression;
}

/// <summary>
/// Instrument the expression of a switch arm of a switch expression.
/// </summary>
public virtual BoundExpression InstrumentSwitchExpressionArmExpression(BoundExpression original, BoundExpression rewrittenExpression, SyntheticBoundNodeFactory factory)
{
Debug.Assert(factory != null);
return rewrittenExpression;
}

public virtual BoundStatement InstrumentSwitchBindCasePatternVariables(BoundStatement bindings)
{
return bindings;
Expand Down
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,7 @@ private BoundExpression LowerSwitchExpression(BoundConvertedSwitchExpression nod
sectionBuilder.Add(_factory.Label(arm.Label));
var loweredValue = _localRewriter.VisitExpression(arm.Value);
if (GenerateSequencePoints)
{
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ [](start = 20, length = 1)

If we are going to keep the if at the end, consider keeping braces as well. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All single-line if statements in this source file are without braces. I prefer to maintain that consistency.

In reply to: 410986252 [](ancestors = 410986252)

// Should go through this._localRewriter._instrumenter; see https://github.com/dotnet/roslyn/issues/42810
loweredValue = new BoundSequencePointExpression(arm.Value.Syntax, loweredValue, loweredValue.Type);
}
loweredValue = this._localRewriter._instrumenter.InstrumentSwitchExpressionArmExpression(arm.Value, loweredValue, _factory);
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loweredValue = this._localRewriter._instrumenter.InstrumentSwitchExpressionArmExpression(arm.Value, loweredValue, _factory); [](start = 24, length = 124)

It is not clear why instrumentation would be linked to generation of sequence points. #Closed

Copy link
Member Author

@gafter gafter Apr 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you prefer I will rename GenerateSequencePoints to GenerateInstrumentation. #Resolved


sectionBuilder.Add(_factory.Assignment(_factory.Local(resultTemp), loweredValue));
sectionBuilder.Add(_factory.Goto(afterSwitchExpression));
Expand Down