Breakpoints on pipeline stages

[dsyme]

This adds breakpoints and stepping on stages in F# forward piping in debug code.

Spec (debug codegen only)

Breakpoint validity

For expr |> f1 |> f2 ... |> fLast , a breakpoint is valid on the expression that is an input to a pipeline, and any "stage" of a pipeline.

Similarly, for (expr1, expr2) ||> f1 |> f2 ... |> fLast and (expr1, expr2, expr3) |||> f1 |> f2 ... |> fLast` , a breakpoint is valid on each of the input expressions, and any "stage" of a pipeline.

Sequence point emit

A corresponding sequence point is emitted in .NET IL before the evaluation of expr and before applying each of f1, f2 etc.

Locals

• A local value Pipe #m input #n @ line is shown within f1, f2, f3, ...for the corresponding value of expr. The pipe number is the occurrence of piping within the method. The n it the input number (elided if only one input). The line is the line number where the input expression occurs.

• A local value Pipe #m stage 1 @ line is shown within f2, f3, ... for the corresponding value of expr |> f1

• A local value Pipe #m stage 2 @ line is shown within f3, ... for the corresponding value of expr |> f1 |> f2

• etc.

The names of the locals are emitted in IL code and are not localized (as that would make compiler output dependent on localization).

Sample screen capture below (stepping) - the local values names differ slightly from the above spec

a.Debugging.-.Microsoft.Visual.Studio.Administrator.2021-08-12.15-17-03.mp4

And with breakpoints:

a.-.Microsoft.Visual.Studio.Administrator.2021-08-12.15-54-23.mp4

[dsyme]

Some unresolved questions:

• This increases the number of locals we emit, e.g. is this a problem in methods with massive pipelines

• The same local names are used regardless of pipeline. When there are sub-pipelines-within-pipelines this may cause confusion. However it's probably a pricce worth paying.

• Breakpoints and sequence points suddenly appearing on innocuous uses of pipelines outside the statement structure, e.g. do we put down debug sequence and breakpoints for pipelines in argument position? Currently in this PR we do. That's OK I guess but may be a bit surprising when stepping. e.g. consider

  someFunction (x |> someThing) (x |> someOtherThing) 

[baronfel]

As far as the names generated for locals in pipelines, IIRC the C# expression evaluator has issues with shadowed names, is it possible to generate unique names (or close-to-unique?) for the intermediate values to sidestep this?

[cartermp]

@dsyme you can debug creating a completion list as a decent test:

fsharp/src/fsharp/service/ServiceDeclarationLists.fs

Line 956 in 8a62ed9

static member Create(infoReader:InfoReader, m: range, denv, getAccessibility: Item -> FSharpAccessibility, items: CompletionItem list, currentNamespace: string[] option, isAttributeApplicationContext: bool) =

[dsyme]

As far as the names generated for locals in pipelines, IIRC the C# expression evaluator has issues with shadowed names,

It does, though as things stand the names aren't actually usable in expression evaluation (<pipe-input> etc.). Not sure what to do about this. If we choose usable names they will potentially conflict with user identifiers. If we don't they can't be used in expression evaluation watch window etc.

is it possible to generate unique names (or close-to-unique?) for the intermediate values to sidestep this?

We could in theory generate unique names (pipe1-input, pipe1-stage1 etc?), though I do wonder if it would be more confusing.

[baronfel]

the names aren't actually usable in expression evaluation

Great point. In VSCode at least (unsure about VS) method returns do have a local that's rendered something like:

MEMBER_NAME returned: RETURN_VALUE

so there might be an option there for us:

PIPELINE_STAGE_TEXT yielded: STAGE_INTERMEDIATE_VALUE

[Happypig375]

If the expression evaluator can support F#, then we can use ``pipe@input`` because warning FS1104: Identifiers containing '@' are reserved for use in F# code generation

[dsyme]

Folks, could you choose between these as names for generated locals please? I need some guidance here:

Option 1 - succinct

pipe1-input 
pipe1-stage1
pipe2-stage2

Option 2 - usable in C# expression evaluator but confusing because they look like F# values

pipe1_input 
pipe1_stage1
pipe2_stage2

Option 3 - uppercase start, spaces with text

Pipe #1 input
Pipe #1 stage #1
Pipe #1 stage #2

Option 4 - something else, please spec

[auduchinok]

If we choose usable names they will potentially conflict with user identifiers.

Can we generate usable names in a similar way to how type parameter names are generated, i.e. keep track of used names? Or, say, similar to function parameter names.

All local names are known at this stage of a compilation, aren't they?

[dsyme]

Can we generate usable names in a similar way to how type parameter names are generated, i.e. keep track of used names? Or, say, similar to function parameter names.

I mean, we could. But I think a bigger problem is cognitive - people look at "pipe1_input" and think it's an F# identifier and wonder what the heck it is. So I suspect that, like return values from functions, it's better to produce text like "Pipe #1 input" or "pipe1-input"

[baronfel]

I agree, I think the names should unambiguously not be F# identifiers if possible. So Option 3 would be my ideal of the ones presented so far.

[auduchinok]

Can this technique also be used in other expressions with implicit "parameters"? If yes, it'd be great to take cases like function or try ... with expressions into account when choosing the naming scheme.

[auduchinok]

But I think a bigger problem is cognitive - people look at "pipe1_input" and think it's an F# identifier and wonder what the heck it is.

Has it been problem for arg1 and similar identifiers before? If not, I'd think it'll be fine here too.

On the other hand, I'd prefer being able to use these identifiers in evaluating expressions in debugger over anything else, so I think I'm slightly biased. I really think evaluating expressions is an important part here. 🙂

[dsyme]

Has it been problem for arg1 and similar identifiers before? If not, I'd think it'll be fine here too.

Yes, any of those identifiers are sloppy and poor tooling I think. I don't want to repeat that

[Happypig375]

If we leave the option for future F# support in expression evaluators, I'd say

pipe1@input 
pipe1@stage1
pipe2@stage2

Because @ is already reserved for identifier names.

[dsyme]

Because @ is already reserved for identifier names.

This symbol isn't reserved for identifier names

[smoothdeveloper]

@dsyme

Back pipes will not be supported for this. They should be used very rarely (see my talk "F# Code I love"), and are therefore not a priority for tooling like this.

Sounds a bit like "security through obscurity" to me, and if the engineering efforts are similar to other operators, I'd find it preferable to enable similar debugging experience, even if the operator is not favored.

Improving debugging experience, especially in messy codebases is a different concern from "code should be nice at all times".

@baronfel / @dsyme

is it possible to generate unique names (or close-to-unique?) for the intermediate values to sidestep this?

We could in theory generate unique names (pipe1-input, pipe1-stage1 etc?), though I do wonder if it would be more confusing.

Despite the shadowing is indeed confusing in that context, I think the generated names won't help, and it will reduce the incentive for debugger infrastructure to ever improve to support shadowed names.

IIRC, in the "locals" tab, it is possible to see all the different symbols, despite they share the same identifier, so I'd be in favour of keeping status quo and tracking the issue upstream on the debugger side, even if things stay as they are.

[dsyme]

I'd find it preferable to enable similar debugging experience, even if the operator is not favored.

Part of the problem is that in

expr1 <|  expr2

The evaluation order is

1. expr1
2. expr2
3. apply result-of-expr1 to result-of-expr2

This doesn't lead to a great natural stepping/breakpoint sequence. Presumably the steps would cover the following ranges:

1. expr1 (covers expr1)
2. expr2 (covers expr2)
3. apply result-of-expr1 to result-of-expr2 (covers whole expression)

This will result in a lot of leaping back and forth, and when placing a breakpoint it will be somewhat random what you get

In contrast, for

expr1 |>  expr2

The evaluation order is

1. expr1
2. expr2
3. apply result-of-expr2 to result-of-expr1

This gives a simplfied two-step natural stepping/breakpoint sequence down the pipeline:

1. stop before evaluating expr1 (covers expr1)
2. stop before evaluating expr2 and applying result-of-expr2 to result-of-expr1 (covers expr2)

This is because x |> f1 |> f2 is sort of more natural in flow

As an aside, in debug code the separation of stages down the pipeline may potentially create more closures, I'm not sure, I will check. It doesn't necessarily have to but it is important to check.

[auduchinok]

Yes, any of those identifiers are sloppy and poor tooling I think. I don't want to repeat that

These identifiers are already seen in lots of places in functions/patterns when debugging F# code, and I assumed people learn what they are or simply ignore them. Having a similar naming would at least be consistent (which I believe helps in learning a language) and, arguably, won't make things worse than they already are.

[dsyme]

Won't make things worse than they already are.

I want to go over the whole lot and make them better :) That is, I'm very open to discussing what "better" is! :)

[auduchinok]

That is, I'm very open to discussing what "better" is! :)

Not that serious: in debug mode, we could wrap these values into structs that would have a good debugger presentation (explaining what they are) and a name that would be accessible in evaluations. 🙂

[baronfel]

Initial look in Ionide is looking pretty good! I can't go to the source directly because of an error in the debugger (which wouldn't be a problem if I were using code I had built, etc, etc), but in the stack frames we can easily see the nice new locals:

Here's an example of what I was talking about, how the .Net Core Debugger in VSCode seems to auto-supply member return values in the locals view. This was taken I think in the range expression in the second pipeline [0..2].

Midway through the second pipeline in the sample:

[zanaptak]

I wonder if the pipe symbol itself could work for generated locals, |> input: or Into |> or something.

[auduchinok]

Option 3 - uppercase start, spaces with text

Pipe #1 input
Pipe #1 stage #1
Pipe #1 stage #2

@dsyme What do you think about having the pipe start line number instead of or in addition to the pipe number? The line where a pipe starts might be a bit more helpful than the number of the pipe.

[Happypig375]

And to be consistent with stack traces for lambdas as well as for future usage from F# support in the debugger evaluator, we can use @ in the shown name to indicate the line number.

[dsyme]

@dsyme What do you think about having the pipe start line number instead of or in addition to the pipe number? The line where a pipe starts might be a bit more helpful than the number of the pipe.

This seems reasonable

And to be consistent with stack traces for lambdas as well as for future usage from F# support in the debugger evaluator, we can use @ in the shown name to indicate the line number.

Perhaps, it seems ok, though we could consider changing that to something clearer, or use C#-like naming for closure classes

[baronfel]

Here's a short gif video of vscode debugging stepping through this feature at the current commit:

The major takeaway is that things mostly Just Work, with some oddities around how putting breakpoints in the lambdas work. You can see the breakpoints at the start of the line bind to the first expression in the lambda instead of the fancy new points, but that is likelyt o be out of our control in Ionide. Luckily when you step out the lambda you get the good pipeline locals again.

[dsyme]

Here's a short gif video of vscode debugging stepping through this feature at the current commit:

...but that is likelyt o be out of our control in Ionide...

Is Ionide using FCS ValidateBreakpointLocation for the language service? If not:

• ValidateBreakpointLocation is used to lay down breakpoints (takes a postion to a range) before debugging is launched
• The actual debugger PDB information is used during debugging

VBPL should be exposed as part of the language server protocol and AKAIK should be a standard sort of thing for languages to implement in VS Code.

[baronfel]

There's a separate specification for debugging in VSCode and it does have a request for setting a breakpoint, but in Ionide we are completely reliant on the executable provided by microsoft that implements this protocol. This is shipped and activated in Omnisharp and is closed source, so I personally have zero visibility into its capabilities or extensibility.

[dsyme]

I've added systematic tests for ValidateBreakpointLocation and this is ready.

[dsyme]

There's a separate specification for debugging in VSCode and it does have a request for setting a breakpoint, but in Ionide we are completely reliant on the executable provided by microsoft that implements this protocol. This is shipped and activated in Omnisharp and is closed source, so I personally have zero visibility into its capabilities or extensibility.

Yes, ultimately we will need our own F# implementation of this - otherwise the debug experience (before launching debug) will be messed up. It won't be a complex thing - presumably we can look at the TypeScript or other implementations.

[TIHan]

Looks like a big change, but it's relatively small with a large amount of impact. :) - Most of the changes in the PR are updating the baseline tests.

I only had a minor nit on the name menv.