[Proposal] Extended mode for sequence point generation

[krasnotsvetov]

Proposal:

Add new modes for sequence point generation (new compiler option).

The main goals are:

• Provide a possibility to implement a condition coverage visualization for runtime instrumentation without sources.
• Improve the debug experience with:
  • conditions (while, if, for)
  • lambda-expression ( * => condition)

Pluses:

1. Improved debug experience
2. Improved testing tools (coverage tools)

   Minuses:

3. The Visual Studio's debugger is skipping some breakpoints after implementation this feature.

You can find more details below with some examples and statistics.

Background:

While I was researching some problems, I noticed, that I could not find a solution without a new information in pdb files, more accurately, without adding new sequence points. More than, I notice, if we add them, then we will provide a better experience with debugging some syntax constructions and provide an additional understanding of evaluation order in their programs.

Let's consider: if (A && B || C) where A, B, C has ExpressionSyntax type.

1-st example: Let's suppose that B throws an exception. Currently, if we put an breakpoint at if's line and then press f10 we will see a dialog with an exception, but we don't know which operand (A, B or C) throws it.

But let's consider equivalent code:

if (A) 
{
   if (B) {
      //then body
   } else if (C) {
      //then body
   }
} else if (C) {
      //then body
}

It doesn't have same problem.

If we wrap A by BoundSequencePoint to BoundSequencePoint(A), B to BoundSequencePoint(B) and C to BoundSequencePoint(C) we will allow debugger to visit each operand and user will have a more pleasant way to find an error.

More than, the user will have an possibility to see a correct evaluation order in current debug session.

2nd example Currently, tools for code coverage which can generate report file without sources (in console mode) like dotCover, OpenCover e.t.c. and then visualize the coverage in Visual Studio don't have an possibility to visualize a coverage metric which is known as "condition coverage". Sometimes we want to see a reason why our code is not covered and where, but we will have only percentages or "yellow" statement without any additional information. Why is it impossible to provide more information?

Let's consider our example again : if (A && B || C), let A was evaluated to false, so B was not evaluated , and the result will depend on C

Condition coverage would allow to show that (a figure below). It is very useful in a correct algorithms with different cases, because a lot of bugs appeared in wrong branches.

Let's consider an il code for this condition:

IL_0000: call
IL_0005: brfalse.s    IL_000e
IL_0007: call
IL_000c: brtrue.s     IL_0015
IL_000e: call
IL_0013: brfalse.s    IL_001f

Currently, we can instrument il code in runtime and only show percentages for condition (66% was covered), but we can't show that A and C was covered and B wasn't because all of them belong to one sequence point (which starts at IL_0000, next starts at IL_0015 and belongs to "then body") and we don't have more information in console mode.

If we wrap A by BoundSequencePoint to BoundSequencePoint(A), B to BoundSequencePoint(B) and C to BoundSequencePoint(C) we will have an interesting result:

• statement coverage will be transformed to condition coverage if we set special flags in compiler(below).

3rd example Sometimes we have an lambda-expressions like * => Expression, where Expression is short-circuit expression (with &&, ||) and we don't have an possibility to see the evaluation order to handle some bugs.

Formally:

Proposal: Add new modes for sequence point generation, like:

• default - without any changes
• extended - with some flags:
  • conditions - each operand in condition will be wrapped by sequence point
  • lambda -each operand in lambda ( * => condition) will be wrapped by sequence point
  • full - each operand in short-circuit expression will be wrapped by sequence point

So we will need to add new compiler option.

Backward compatibility:

By default we will use a "default" mode for sequence point generation, so we can't break anything.

Statistics:

Here you can find some statistics how often if statements used with more than 1 operand in condition.

Project name	if statement count	condition's operands > 1	%
Roslyn	~49 500	~10 200	~20
CodeContracts	~27 400	~4 600	~16
DnSpy	~35 500	~5 300	~15
Omnisharp-Roslyn	~660	~75	~11
NopCommerce	~8 600	~1 100	~12
NewtonSoft.Json	~2 600	~420	~16

For lambda-functions:

Project name	lambda	* => condition	lambda with if in body
Roslyn	~12 000	~500	~1 500

Implementation:

One year ago, a set of functions was moved to API with name Instrumenter, and CompoundInstrumenter was appeared. I suggest use this API to implement this feature.

The sequence point generation is not possible everywhere, so we can't make this API public, so we should:

• add new compiler options
• choose a CompoundInstrumenter which will satisfied a new compiler option

For example, if we want to implement extended-condition we should create a new class which will be inherited from DebugInfoInjector and its second part and override this method:

• InstrumentIfStatementCondition
  • and don't forget to update LocalRewriter, because, for example, LocalRewriter_IfStatement generates a sequence point in place where is the first condition's operand starts.

We can transform short-circuit expression via this rule (Transform):

• BinaryOperation(&& or ||, A, B) → BinaryOperation(&& or ||, Transform(A), Transform(B))
• UnaryOperation(!, A) → UnaryOperation(!, Transform(A)
• Parenthesized(A) → Parenthesized(Transform(A))
• Operand → BoundSequencePoint(Operand)

In particular I have this implementation:

BoundExpression AddAdditionalSequencePointForCondition(BoundExpression expression)
{
      if (expression is BoundBinaryOperator bbo && (bbo.OperatorKind == BinaryOperatorKind.LogicalBoolOr 
          || bbo.OperatorKind == BinaryOperatorKind.LogicalBoolAnd))
      {
          return new BoundBinaryOperator(bbo.Syntax, bbo.OperatorKind, AddAdditionalSequencePointForCondition(bbo.Left), 
              AddAdditionalSequencePointForCondition(bbo.Right), bbo.ConstantValueOpt, bbo.MethodOpt, bbo.ResultKind,
              bbo.OriginalUserDefinedOperatorsOpt, bbo.Type, bbo.HasErrors);
      }
      else
      {
          return new BoundSequencePointExpression(expression.Syntax, expression, expression.Type,
             expression.HasErrors);
      }
    }

Problems:

Currently, I have an primitive implementation (function above) and I noticed some problem, that Visual Studio's debugger skips some sequence point (only if statements) while I was stepping (f10, f11) if they contain il instructions like:

• call / calli / callvirt
• newobj

[gafter]

/cc @tmat FYI

[gafter]

Placed in the 16.0 milestone for triage for that future release.