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.

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

[Proposal] Extended mode for sequence point generation #21781

Open
krasnotsvetov opened this issue Aug 29, 2017 · 2 comments
Open

[Proposal] Extended mode for sequence point generation #21781

krasnotsvetov opened this issue Aug 29, 2017 · 2 comments

Comments

@krasnotsvetov
Copy link

krasnotsvetov commented Aug 29, 2017

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:

  1. 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.

debugger

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).

image

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:

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
Copy link
Member

gafter commented Sep 15, 2017

/cc @tmat FYI

@gafter gafter added this to the 16.0 milestone Sep 15, 2017
@gafter
Copy link
Member

gafter commented Sep 15, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants