Allow setting breakpoints on throw expressions

[sharwell]

Version Used: 15.2

Steps to Reproduce:

1. Add the following code

   void Method(object a, object b)
   {
     if (a == null)
       throw new ArgumentNullException(nameof(a));
   
     var tmpA = a;
     var tmpB = b ?? throw new ArgumentNullException(nameof(b));
   }

2. Add a breakpoint which breaks into the debugger immediately before a call to new ArgumentNullException("a")

3. Add a breakpoint which breaks into the debugger immediately before a call to new ArgumentNullException("b")

Expected Behavior:

The debugging experience is not impaired by a decision to use throw expressions to simplify code.

Actual Behavior:

The debugging experience is substantially impaired:

• Simplicity: The breakpoint for b being null requires a conditional breakpoint with a manually-typed condition. It used multiple UI elements, further disrupting the workflow of a user attempting to add it.
• Performance: The conditional breakpoint for b being null is very slow to evaluate compared to the unconditional breakpoint used for a being null.

[svick]

What makes the throw expression special? If you're going to allow setting breakpoint on the throw expression, why not also allow all the other kinds of expressions?

[AdamSpeight2008]

@svick It's not special but it setting a breakpoint on InvocationExpression would cover a lot the use cases.

[tmat]

Since similar feature request have been made previously I have filed a more general issue to track feature of placing breakpoint on an arbitrary expression: https://github.com/dotnet/roslyn/issues/22016. I would not hold my breath though expecting this feature to be implemented any time soon.

[sharwell]

@svick The throw expression is the only point of direct return from a method on which a breakpoint cannot be set.

@tmat I'm not nearly as interested in the general feature. Setting breakpoints on throw statements has regularly proven to be specifically valuable in debugging scenarios. It was largely the motivation for this question and later the Java debugger I wrote. Regularly-encountered problems debugging these cases also formed by particularly strong code style stance against single-line if/throw argument validation checks in Java (where debuggers cannot distinguish two statements on the same line).

Long-standing debugging experience with argument validation and real-world projects leads me to classify throw expression breakpoints as a failure to reach feature parity, while other subexpression breakpoints would simply be a new feature request. I would request that this issue be reopened with notably higher priority than the general handling of expressions (which may end up rejected for other negative usability impact).

[sharwell]

@tmat I'm reopening this. Exceptions represent a special case in flow control which result in leaving a method. We don't need the general ability to set breakpoints in subexpressions, but we need the ability to set breakpoints on a throw expression.

[CyrusNajmabadi]

i agree with sam. It seems like we just need this on throw-expressions. I'm curious what level of hte stack would be blocking this. Is it just the IDE side? Or are we not creating hte right PDB information in order to actually do this properly in the debugger?

[tmat]

The PDB, EnC, maybe JIT, since C# has never emitted sequence points for non-statements, so it's not tested.

[CyrusNajmabadi]

I'd be surprised if the pdb/jit cared. There's no actual difference at the IL/jit level for throw expressions/statements right? i mean, we're just going to convert x ?? throw y into:

if (x == null)
{
    throw y;
}

as far as the rest of the system is concerned.

Right?

[tmat]

@CyrusNajmabadi The stack might not be empty:

F(G(), x ? y : throw expr);

[CyrusNajmabadi]

I don't quite understand. Isn't that just:

var __temp1 = G();
if (!x) { throw expr };
F(__temp1, y);

?

[tmat]

No, we don't do stack spilling.

class C
{
   static string F(string a, string b, string c) => a;

   public static void Main()
   {
      string a = "1";
      string b = "2";
      string c = "3";

      F(
         a ?? throw new Exception(),
         b ?? throw new Exception(),
         c ?? throw new Exception()
      );
   }
}

Compiles to

IL_0000:  nop
  IL_0001:  ldstr      "1"
  IL_0006:  stloc.0
  IL_0007:  ldstr      "2"
  IL_000c:  stloc.1
  IL_000d:  ldstr      "3"
  IL_0012:  stloc.2
  IL_0013:  ldloc.0
  IL_0014:  dup
  IL_0015:  brtrue.s   IL_001e
  IL_0017:  pop
  IL_0018:  newobj     instance void [mscorlib]System.Exception::.ctor()
  IL_001d:  throw
  IL_001e:  ldloc.1
  IL_001f:  dup
  IL_0020:  brtrue.s   IL_0029
  IL_0022:  pop
  IL_0023:  newobj     instance void [mscorlib]System.Exception::.ctor()
  IL_0028:  throw
  IL_0029:  ldloc.2
  IL_002a:  dup
  IL_002b:  brtrue.s   IL_0034
  IL_002d:  pop
  IL_002e:  newobj     instance void [mscorlib]System.Exception::.ctor()
  IL_0033:  throw
  IL_0034:  call       string C::F(string,
                                   string,
                                   string)
  IL_0039:  pop
  IL_003a:  ret

[jnm2]

Would be covered by https://github.com/dotnet/roslyn/issues/43092 which @gafter asked me to open.