src/debugAdapter: add delve 'call' command support
Delve supports function calls. Even though it is still
experimental and can be applied only to a limited set
of functions, this is a useful feature, many vscode-go
users long for.
Unlike other javascript/typescript debuggers, delve
treats function calls specially and requires different
call paths than usual expression evaluation. That is
because Go is a compiled, runtime-managed GC language,
calling a function safely from debugger is complex.
DAP and VS Code UI does not distinguish function calls
and other expression evaluation either, so we have to
implement this in the same `evaluateRequest` context.
We use a heuristic to guess which route (call or
expression evaluation) we need to take based on
evaluateRequest's request.
The set of expressions delve supports includes some of
the builtin function calls like `len`, `cap`. We also
expect some grammar changes due to the ongoing effort for
Go2 experiment. In order to cope with this uncertainty,
this change requires users to specify their intention
using the 'call' keyword. Any expression that starts with
'call' and contains function-call-like strings, will cause
the evaluateRequest to take the path for the `call` command.
And, DAP and VSCode assumes all expressions return only
one result, but in Go, function call can return multiple
values. In this CL, we work around this sementic difference
by wrapping all the return values in one wrapper result
that has all return values as children. Users can expand
each value.
While we are here, this CL also adds a logic to handle the
interface type value. Previously, it was captured by the
default case that does not show the underlying type and value.
Interface types behave differently depending on the underlying
data type and value (e.g. nil error https://golang.org/doc/faq#nil_error)
so showing the underlying type/value upfront helps debugging
much easier. In particular, the 'error' type is commonly used
as the last return value of a function call and the program
control flow changes depending on whether the error is nil.
So, here we surface the underlying type and, if the underlying
type is invalid and the addr is 0 (nil error), present the
result as 'nil <error>'. This allows users to recognize the 'nil'
error value without extra value expansion.
Any error with an underlying type will be presented as
`<error(someUnderlyingType)>` and require the value expansion.
I see we can further improve this display value handling and
try to follow delve's data presentation as much as possible but
that is a bigger change and is beyond the scope of this CL.
This is based on github.com/golang/vscode-go/pull/101
Fixes golang/vscode-go#100
Co-authored-by: Eugene Kulabuhov <eugene.kulabuhov@gmail.com>
Change-Id: I4e1e876ab1582b17fa2c7bf0b911f31ec348b484
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/249377
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Polina Sokolova <polina@google.com>
diff --git a/src/debugAdapter/goDebug.ts b/src/debugAdapter/goDebug.ts
index f669ac5..3ade4b9 100644
--- a/src/debugAdapter/goDebug.ts
+++ b/src/debugAdapter/goDebug.ts
@@ -37,7 +37,7 @@
getCurrentGoWorkspaceFromGOPATH,
getInferredGopath,
} from '../utils/goPath';
-import {killProcessTree} from '../utils/processUtils';
+import { killProcessTree } from '../utils/processUtils';
const fsAccess = util.promisify(fs.access);
const fsUnlink = util.promisify(fs.unlink);
@@ -150,6 +150,7 @@
pc: number;
goroutineID: number;
function?: DebugFunction;
+ ReturnValues: DebugVariable[];
}
interface StacktraceOut {
@@ -190,10 +191,13 @@
VariableShadowed = 2,
VariableConstant = 4,
VariableArgument = 8,
- VariableReturnArgument = 16
+ VariableReturnArgument = 16,
+ VariableFakeAddress = 32
}
interface DebugVariable {
+ // DebugVariable corresponds to api.Variable in Delve API.
+ // https://github.com/go-delve/delve/blob/328cf87808822693dc611591519689dcd42696a3/service/api/types.go#L239-L284
name: string;
addr: number;
type: string;
@@ -1276,6 +1280,10 @@
protected scopesRequest(response: DebugProtocol.ScopesResponse, args: DebugProtocol.ScopesArguments): void {
log('ScopesRequest');
+ // TODO(polinasok): this.stackFrameHandles.get should succeed as long as DA
+ // clients behaves well. Find the documentation around stack frame management
+ // and in case of a failure caused by misbehavior, consider to indicate it
+ // in the error response.
const [goroutineId, frameId] = this.stackFrameHandles.get(args.frameId);
const listLocalVarsIn = { goroutineID: goroutineId, frame: frameId };
this.delve.call<DebugVariable[] | ListVarsOut>(
@@ -1589,8 +1597,47 @@
log('PauseResponse');
}
+ // evaluateRequest is used both for the traditional expression evaluation
+ // (https://github.com/go-delve/delve/blob/master/Documentation/cli/expr.md) and
+ // for the 'call' command support.
+ // If the args.expression starts with the 'call' keyword followed by an expression that looks
+ // like a function call, the request is interpreted as a 'call' command request,
+ // and otherwise, interpreted as `print` command equivalent with RPCServer.Eval.
protected evaluateRequest(response: DebugProtocol.EvaluateResponse, args: DebugProtocol.EvaluateArguments): void {
log('EvaluateRequest');
+ // Captures pattern that looks like the expression that starts with `call<space>`
+ // command call. This is supported only with APIv2.
+ const isCallCommand = args.expression.match(/^\s*call\s+\S+/);
+ if (!this.delve.isApiV1 && isCallCommand) {
+ this.evaluateCallImpl(args).then((out) => {
+ const state = (<CommandOut>out).State;
+ const returnValues = state?.currentThread?.ReturnValues ?? [];
+ switch (returnValues.length) {
+ case 0:
+ response.body = { result: '', variablesReference: 0 };
+ break;
+ case 1:
+ response.body = this.convertDebugVariableToProtocolVariable(returnValues[0]);
+ break;
+ default:
+ // Go function can return multiple return values while
+ // DAP EvaluateResponse assumes a single result with possibly
+ // multiple children. So, create a fake DebugVariable
+ // that has all the results as children.
+ const returnResults = this.wrapReturnVars(returnValues);
+ response.body = this.convertDebugVariableToProtocolVariable(returnResults);
+ break;
+ }
+ this.sendResponse(response);
+ log('EvaluateCallResponse');
+ }, (err) => {
+ this.sendErrorResponse(response, 2009, 'Unable to complete call: "{e}"', {
+ e: err.toString()
+ }, args.context === 'watch' ? null : ErrorDestination.User);
+ });
+ return;
+ }
+ // Now handle it as a conventional evaluateRequest.
this.evaluateRequestImpl(args).then(
(out) => {
const variable = this.delve.isApiV1 ? <DebugVariable>out : (<EvalOut>out).Variable;
@@ -1601,20 +1648,15 @@
log('EvaluateResponse');
},
(err) => {
- let dest: ErrorDestination;
// No need to repeatedly show the error pop-up when expressions
// are continiously reevaluated in the Watch panel, which
// already displays errors.
- if (args.context === 'watch') {
- dest = null;
- } else {
- dest = ErrorDestination.User;
- }
this.sendErrorResponse(response, 2009, 'Unable to eval expression: "{e}"', {
e: err.toString()
- }, dest);
+ }, args.context === 'watch' ? null : ErrorDestination.User);
}
);
+
}
protected setVariableRequest(
@@ -1950,6 +1992,41 @@
});
}
+ // Go might return more than one result while DAP and VS Code do not support
+ // such scenario but assume one single result. So, wrap all return variables
+ // in one made-up, nameless, invalid variable. This is similar to how scopes
+ // are represented. This assumes the vars are the ordered list of return
+ // values from a function call.
+ private wrapReturnVars(vars: DebugVariable[]): DebugVariable {
+ // VS Code uses the value property of the DebugVariable
+ // when displaying it. So let's formulate it in a user friendly way
+ // as if they look like a list of multiple values.
+ // Note: we use only convertDebugVariableToProtocolVariable's result,
+ // which means we will leak the variable references until the handle
+ // map is cleared. Assuming the number of return parameters is handful,
+ // this waste shouldn't be significant.
+ const values = vars.map((v) => this.convertDebugVariableToProtocolVariable(v).result) || [];
+ return {
+ value: values.join(', '),
+ kind: GoReflectKind.Invalid,
+ flags: GoVariableFlags.VariableFakeAddress | GoVariableFlags.VariableReturnArgument,
+ children: vars,
+
+ // DebugVariable requires the following fields.
+ name: '',
+ addr: 0,
+ type: '',
+ realType: '',
+ onlyAddr: false,
+ DeclLine: 0,
+ len: 0,
+ cap: 0,
+ unreadable: '',
+ base: 0,
+ fullyQualifiedName: ''
+ };
+ }
+
private convertDebugVariableToProtocolVariable(v: DebugVariable): { result: string; variablesReference: number } {
if (v.kind === GoReflectKind.UnsafePointer) {
return {
@@ -2017,6 +2094,35 @@
result: v.unreadable ? '<' + v.unreadable + '>' : '"' + val + '"',
variablesReference: 0
};
+ } else if (v.kind === GoReflectKind.Interface) {
+ if (v.addr === 0) {
+ // an escaped interface variable that points to nil, this shouldn't
+ // happen in normal code but can happen if the variable is out of scope.
+ return {
+ result: 'nil',
+ variablesReference: 0
+ };
+ }
+
+ if (v.children.length === 0) { // Shouldn't happen, but to be safe.
+ return {
+ result: 'nil',
+ variablesReference: 0
+ };
+ }
+ const child = v.children[0];
+ if (child.kind === GoReflectKind.Invalid && child.addr === 0) {
+ return {
+ result: `nil <${v.type}>`,
+ variablesReference: 0
+ };
+ }
+ return {
+ // TODO(hyangah): v.value will be useless. consider displaying more info from the child.
+ // https://github.com/go-delve/delve/blob/930fa3b/service/api/prettyprint.go#L106-L124
+ result: v.value || `<${v.type}(${child.type})>)`,
+ variablesReference: v.children?.length > 0 ? this.variableHandles.create(v) : 0
+ };
} else {
// Default case - structs
if (v.children.length > 0) {
@@ -2103,13 +2209,51 @@
return this.delve.callPromise('Command', [{ name: 'continue' }]).then(callback, errorCallback);
}
+ // evaluateCallImpl expects args.expression starts with the 'call ' command.
+ private evaluateCallImpl(args: DebugProtocol.EvaluateArguments)
+ : Thenable<DebuggerState | CommandOut> {
+ const callExpr = args.expression.trimLeft().slice(`call `.length);
+ // if args.frameID is 'not specified', expression is evaluated in the global scope, according to DAP.
+ // default to the topmost stack frame of the current goroutine
+ let goroutineId = -1;
+ let frameId = 0;
+ if (args.frameId) {
+ [goroutineId, frameId] = this.stackFrameHandles.get(args.frameId, [goroutineId, frameId]);
+ }
+ // See https://github.com/go-delve/delve/blob/328cf87808822693dc611591519689dcd42696a3/service/api/types.go#L321-L350
+ // for the command args for function call.
+ const returnValue = this.delve
+ .callPromise<DebuggerState | CommandOut>('Command', [
+ {
+ name: 'call',
+ goroutineID: goroutineId,
+ returnInfoLoadConfig: this.delve.loadConfig,
+ expr: callExpr,
+ unsafe: false,
+ }
+ ])
+ .then(
+ (val) => val,
+ (err) => {
+ logError(
+ 'Failed to call function: ',
+ JSON.stringify(callExpr, null, ' '),
+ '\n\rCall error:',
+ err.toString()
+ );
+ return Promise.reject(err);
+ }
+ );
+ return returnValue;
+ }
+
private evaluateRequestImpl(args: DebugProtocol.EvaluateArguments): Thenable<EvalOut | DebugVariable> {
// default to the topmost stack frame of the current goroutine
let goroutineId = -1;
let frameId = 0;
// args.frameId won't be specified when evaluating global vars
if (args.frameId) {
- [goroutineId, frameId] = this.stackFrameHandles.get(args.frameId);
+ [goroutineId, frameId] = this.stackFrameHandles.get(args.frameId, [goroutineId, frameId]);
}
const scope = {
goroutineID: goroutineId,
