service/dap: auto-loading for fully missing children of nested vars

[polinasok]

This change provides support for automatically reloading fully missing pointers, structs, arrays and maps (subject to partial config limits on reload). It does not attempt to reload partially loaded variables, which need a different way of handling (TBD).

This change reuses the code used for auto-loading interfaces, factored out into a helper reloadVariable. Since VariablesRequest focuses on looking up already loaded variables, we auto-load the children proactively when looking up their outer container variable. This allows us to update the inlined string value of the parent (but not grandparent) variable as well and avoid supplying references that resolve to nothing if auto-loading fails.

Pointers loading has a couple of quirks comparing to other types. If a pointer was not loaded because FollowPointers=false (hypothetical for now because users have no way of altering this, but that could change if we merge this server with the rpc server), then we must flip FollowPointers=true before reloading or surgically reload just the child data. Also discovered a new pointer use case - &, a special case in delve that returns a usable pointer-expression, which is not loaded by default even with FollowPointers=true. Because this case corresponds to 0x0 pointer address, it cannot be loaded directly, but instead by reloading its child data only. Should it be at all?

This is an alternative implementation to #2453 (see comment).

Updates #1515
Updates golang/vscode-go#1052
Updates golang/vscode-go#1450

[polinasok]

Full disclaimer: I am not quite happy with how the support for pointers looks like right now. Detailed thoughts are in the code.

[polinasok]

cc @hyangah

[hyangah]

I am happy to abandon #2453 if you want.

A couple of comments

• In this PR, it looks like the partially or unloaded child causes prefetching (loading with EvalVariableInScope). On the other hand, I tried to avoid the prefetching until user triggers another variables request (so I didn't care about the FollowPointers setting - user's request implies the user wants to follow the pointer).
• screenshot may be helpful to show how the expansion through reloading is presented.
• may be want to keep track of the frame/goroutine? Or is it unnecessary?

[polinasok]

Thank you for a quick response!

• In this PR, it looks like the partially or unloaded child causes prefetching (loading with EvalVariableInScope).

Only fully missing for now. Partial ones are untouched in his change.

On the other hand, I tried to avoid the prefetching until user triggers another variables request

I played with both options before sending out a prototype for interfaces and decided against that approach.
The dap protocol is set up such that you load and communicate back information on the child variables (i.e. prefetch them) when responding to a Variables request for the parent container. So this is consistent with the protocol. And this way we can actually have a valid inlined value that's reflecting the additional loading. If you load later, you won't be able to update it in the UI anymore. With your approach you have to remove the "loaded x/y" labels and add expansion toggle before you actually know that things can be loaded.

(so I didn't care about the FollowPointers setting - user's request implies the user wants to follow the pointer).

The updated test shows no evidence that your approach worked with FollowPointers=false because the value is still the old unloaded one (minus the "not loaded" label) and while you flipped noChildren=>hasChildren, you have not actually loaded those children in the test, so I don't know if the auto-loading was successful. From reading the code it seems that you would have he same issue because you mark the pointer, not the data child as partial, so when the parent is looked up, you try to reload it, which would still be subject to FollowPointers=false.

• screenshot may be helpful to show how the expansion through reloading is presented.

Will do this next.

• may be want to keep track of the frame/goroutine? Or is it unnecessary?

It is not necessary if we use special expressions. There is already a test confirming that we can do auto-loading for any frame.

[polinasok]

Before:

After:

[polinasok]

With call:

[hyangah]

Thanks. As long as prefetching can handle large data structure, a large number of variables in the scope without noticeable performance penalty and still allow navigating deeply nested structures, I am happy. I will drop my PR to stop wasting more time.

[aarzilli]

This is what a client would do, but this code lives in the same process as the debugger and has a (presumed valid) *proc.Variable object, so it can do better. It can set v.loaded to false and call v.loadValue again. None of this is exported at the moment, and it needs to be done while holding a the target mutex.

It's more efficient and it's guaranteed to keep working with generics.

[polinasok]

I copied this code from the previous iteration that you blessed. I am happy to improve, but could we do that in a separate PR please? I added a TODO.

[aarzilli]

Ok.

[derekparker]

LGTM