Skip to content

Commit

Permalink
dap: handle SetVariable requests (go-delve#2440)
Browse files Browse the repository at this point in the history
* dap: handle SetVariable requests

The handler invokes debugger.SetVariableInScope, except for
string type variables. For which, we rely on the `call` command.
Moved the call expression handling logic to the new `doCall`
function, so it can be reused by the SetVariable requenst
handler.

With this PR, every successful SetVariable request triggers
a StoppedEvent - that's a hack to reset the variablesHandle
map internally and notify the client of this change. It will
be nice if we can just update cached data corresponding to
the updated variable.  But I cannot find an easy and safe way
to achieve this yet.

Also fixed a small bug in the call expression evaluation -
Previously, dlv dap returned an error "Unable to evaluate
expression: call stopped" if the call expression is for
variable assignment.  (e.g. "call animal = "rabbit").

* dap: address comments from aarzilli

resetHandlesForStop & sendStoppedEvent unconditionally after
call command is left as a TODO - This is an existing code path
(just refactored) and an preexisting bug. Fixing it here
requires updates in TestEvaluateCallRequest and I prefer
addressing it in a separate cl.

Disabled call injection testing on arm64. Separated TestSetVariable
into two, one that doesn't involve call injection and another that
may involve call injection.

Fixed variableByName by removing unnecessary recursion.

* dap: address polina's comments

- removed the hard reset for every variable set
- added tests for various variable types
- added tests that involves interrupted function calls. (breakpoint/panic)

And,
- changed to utilize EvalVariableInScope to access the variable instead
of searching the children by name.
- changed to utilize evaluate requests when verifying whether the variable
is changed as expected in testing. Since now we avoid resetting the variable
handles after variable reset, either we need to trigger scope changes
explicitly, or stop depending on the variables request.

* dap: address comments

- Discuss the problem around the current doCall implementation
and the implication.
- Refine the description on how VS Code handles after setVariable
and evaluate request (there could be followup scopes/evaluate requests).
- Use the explicit line numbers for breakpoints in the SetVariable tests.
- Do not use errors.Is - we could've used golang.org/x/xerrors polyfill
but that's an additional dependency, and we will remove this check once
tests that depend on old behavior are fixed.

* dap: remove errTerminated and adjust the test

* dap: evaluate in the outer frame, instead of advancing to the next bp
  • Loading branch information
hyangah authored May 20, 2021
1 parent 4f11320 commit c8934dc
Show file tree
Hide file tree
Showing 5 changed files with 638 additions and 93 deletions.
4 changes: 2 additions & 2 deletions _fixtures/testvariables2.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,8 @@ func main() {

var nilstruct *astruct = nil

val := A{val: 1} // val vs val.val
var as2 astruct

s4 := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}

var iface2map interface{} = map[string]interface{}{
Expand Down Expand Up @@ -363,5 +363,5 @@ func main() {
longslice := make([]int, 100, 100)

runtime.Breakpoint()
fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice)
fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val)
}
9 changes: 7 additions & 2 deletions service/dap/daptest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func (c *Client) ExpectInitializeResponseAndCapabilities(t *testing.T) *dap.Init
SupportsDelayedStackTraceLoading: true,
SupportTerminateDebuggee: true,
SupportsExceptionInfoRequest: true,
SupportsSetVariable: true,
SupportsFunctionBreakpoints: true,
SupportsEvaluateForHovers: true,
}
Expand Down Expand Up @@ -353,8 +354,12 @@ func (c *Client) ReverseContinueRequest() {
}

// SetVariableRequest sends a 'setVariable' request.
func (c *Client) SetVariableRequest() {
c.send(&dap.ReverseContinueRequest{Request: *c.newRequest("setVariable")})
func (c *Client) SetVariableRequest(variablesRef int, name, value string) {
request := &dap.SetVariableRequest{Request: *c.newRequest("setVariable")}
request.Arguments.VariablesReference = variablesRef
request.Arguments.Name = name
request.Arguments.Value = value
c.send(request)
}

// RestartFrameRequest sends a 'restartFrame' request.
Expand Down
1 change: 1 addition & 0 deletions service/dap/error_ids.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
UnableToEvaluateExpression = 2009
UnableToHalt = 2010
UnableToGetExceptionInfo = 2011
UnableToSetVariable = 2012
// Add more codes as we support more requests
DebuggeeIsRunning = 4000
DisconnectError = 5000
Expand Down
Loading

0 comments on commit c8934dc

Please sign in to comment.