Debugging and edit-and-continue for pattern-matching: action items

[gafter]

Based on discussion with @tmat we came up with the following set of work items that we expect will provide reasonable debugging and edit-and-continue support for pattern-matching constructs.

Generally speaking, these action items result from the following understanding of the way things work:

• The execution stack must be empty at a sequence point (even a hidden sequence point)
• There should be a sequence point shortly after any hidden call to user code such as a property, Deconstruct, or string.operator==. This becomes an opportunity for the edit-and-continue infrastructure to move the PC from the old method's frame to the new method's frame, mapping variables from one to the other in the process.

Based on this, we have the following action items:

• In a switch when a when clause is on the execution stack, changing any case clause is a rude edit. That's because changing a case clause could potentially invalidate the shape of the lowered decision dag, making it infeasible to map the old code to the new code. This applies to the when clause as well (it is part of the case clause), because a general expression in the when clause it treated differently than a constant true or a constant false expression. We could relax this by not permitting changes to or from a constant-valued when clause.
• If the decision dag code is on the stack (e.g. when stopped in a called property or Deconstruct method), changing any case clause is a rude edit.
• There should be a hidden sequence point at the start of the code to handle the decision dag. This is necessary so that jumps back from a when clause into the decision dag do not appear to jump back up to the switch statement. (Fixed in recursive-patterns(21) Add hidden sequence points for switch statement. #26138)
• After a decision that calls out to user code (e.g. string.operator ==), the result should be saved, leaving the stack empty, then there should be a sequence point, then the result should be loaded again. This bool temp can be shared for all the places this needs to be done.
• After an evaluation that calls out to user code, the decision dag already saves it to a temp. We should add a hidden sequence point there when the stack is empty. (Fixed in recursive-patterns(21) Add hidden sequence points for switch statement. #26138)
• Temps that are used in lowering the decision dag and that need to survive across calls out to user code (i.e. pretty much all of them) should be classified as long-lived.
• Those temps need to be associated with the switch statement's syntax node.
• For a switch expression (a new construct that is an expression form of the switch), in debug, we will need to spill anything that is on the stack at the start of the switch expression, and restore afterwards. Otherwise the stack would not be empty at the sequence points. We may be able to share code with the spilling rewriter currently used to lower async methods.

Given that we need to implement spilling for the switch expression for debug, we might elect to use spilling to simplify the compiler pipeline in release as well.

/cc @tmat @dotnet/roslyn-compiler

[tmat]

Let's have a separate discussion re switch expression. It might be possible to avoid the need for sequence points in that case. I believe sequence points are not strictly necessary for the debugger to be able to place and stop at a breakpoint. We would need to handle such breakpoints differently than breakpoints at sequence points throughout the system (especially in edit and continue).

We have an issue tracking breakpoints on expressions in general: #22016. Switch expression should just be part of that work.

[gafter]

Just as some editing of a containing switch statement is permitted when some part of that switch statement is on the stack, I had assumed that we would want to permit some editing of a switch expression even when some part of it is on the stack.

[tmat]

That's gonna be a bit tricky. We would need to spill and track the spill temps. We do something like that for await spilling but I'd need to investigate if we can reuse the same mechanism for switch expression. I'd definitely track switch expression separately from switch statement as it's gonna be harder.

[gafter]

@tmat @ivanbasov What do you think we need to do for the remaining action items (unchecked bullets) above? Are they must-do for 16.3, or can some of them be deferred to later?
/cc @jcouv

[tmat]

Switch expressions EnC is disabled in 16.3 due to bugs:
#37232
#37261
#37237

We need to revisit how closures are emitted for lifted variables declared in the switch expression.

Added umbrella issue: #43099

[tmat]

I believe all issues have been resolved now.