Proposal: Add new reason `finished` or `stepOut` and optional field `returnValue` to `Stopped Event`

[theIDinside]

As a maintainer of a debug adapter, I've been asked a few times about being able to display return values somehow, and even though it's doable with some minor hacking about (actually implemented sort of like it's described here, thus being non-standard compliant), it would be nice if it got standardized in the protocol.

Since we have stepOut request, which explicitly "finishes" the current function (in GDB lingo), a new reason value should be added to reflect that this has happened, instead of just reporting step. Due to this new reason, an optional field of type Variable should also be added to the StoppedEvent, where a structured representation of the return value is returned.

An explicit stepOut reason I think is to prefer, than just having it remain step and then have an optional value or something like that on StoppedEvent because it doesn't signal as clearly intention. Especially since step stops come in so many forms (next line, next instruction, next statement, etc).

Also, adding an additional stepOut reason, means that legacy adapters can continue signalling step if they so choose if they are unwilling or don't want to support the feature of displaying return values after a stepOut request.

[connor4312]

The purpose of the step reason is to disambiguate the reason if an event interrupts a step that the client previously sent. For example, if I step over an instruction but have a data breakpoint that's hit before we'd normally pause on the next instruction. In cases where step is given as the reason, the client will already know in which direction the step happened, so I'm not sure the value in specifying beyond that.

---

For the return value, the way most DA's do this today is adding the return value to the first scope when the return value is present, for example in VS Code's Python debugger:

If we have an explicit DAP-level return value, is there a better experience you envision that clients would want to implement?

[theIDinside]

I guess this work just as well.

I suppose one way my proposed way would be preferred, is if in some universe, someone decides to log return values from a function (or set of functions). Automating this process would probably be easier if a "return value" was standardized in the protocol from stepOut, but even in that universe, that functionality could be solved using the same approach as you mentioned.

So I guess we can close this issue really.

[connor4312]

Hm, nothing in particular comes to mind for usages of it in VS Code's UI (cc @roblourens) but let's leave it open for a bit to see if any other client implementors want to weigh in.

In general DAP is a protocol for specifying what UI should be available during debugging, so most of our thinking starts at the client experience we want to build, and from there the minimal protocol necessary to describe it.

[puremourning]

I personally find the ‘put the return value in the scopes’ to be a hack that’s only sporadically implemented and not intuitive for users. I would like to offer a better ui to my users.

The obvious UI for this is to inline it as an inlay hint or virtual test next to or around the function call. This is not trivial and I don’t think there’s enough info in the proposal for clients to implement that well.

In order for this to work well we would need it whether the user pressed step out or step over etc. (Parity with the ‘scopes’ approach). It could be enough just to provide a variablesReference and sourceReference/line number or something.

In summary I think it would be a good addition, but needs more iteration on the UI side to decide what data are worthwhile.

Alternatively, a statement that “typically the return value from the last function call should be reported as a Scope named ‘return’” might help with consistency for users.

[tromey]

Alternatively, a statement that “typically the return value from the last function call should be reported as a Scope named ‘return’” might help with consistency for users.

This would definitely be an improvement -- for my gdb implementation, I was completely unaware that this was a convention.

Adding a variable reference to the stop event also seems reasonable to me.

Alternatively I suppose an adapter could also do this via an output event. (Though again, the key thing is to document any such convention.)

[puremourning]

Output event is very bad. Probably not even visible to user in most cases.

[connor4312]

I think this makes sense. However, I suggest that we put it on the stack frame rather than the "stopped". This allows multiple stack frames to represent that they're in the process of returning data. This is semantically sensible in some cases, such as when one is stepped through code called in a finally block after a return from a try block, or with defered code in Go or Zig.

interface StackFrame {
	// ...

	/**
	 * A value being returned by from this stack frame.
	 */
	returnValue?: Variable;

Though not strictly necessary, we should also introduce a client capability so that adapters that have been adding ad-hoc return variables avoid duplicating data:

interface InitializeRequestArguments {
	/**
	 * Client supports reading the `returnValue` from a `StackFrame`.
	 */
	supportStackFrameReturnValue?: boolean;

cc @roblourens

[gregg-miskelly]

A few problems with putting it on the stack frame:

1. Is this supposed to represent the return value from this frame? If so, that doesn't make sense as any frame that has a stack frame hasn't returned yet. If not, then might be many functions that returned recently.
2. Unless we add a new gesture, a debug adapter would be forced to either always calculate it (which might not make sense since these aren't necessarily cheap) or we need a way to say when these are used

My suggestion: these should be in a scope. I understand some folks on this thread have considered this a 'hack' though it doesn't seem like one to me. Would folks consider this less hacky if scope.presentationHint was extended with a value for this?

[connor4312]

uld folks consider this less hacky if scope.presentationHint was extended with a value for this?

I like that idea. That also allows for a graceful fallback for existing clients without introducing any new capabilities. That would be extending the presentationHint on the Scope as such:

interface Scope
  /**
   * A hint for how to present this scope in the UI. If this attribute is
   * missing, the scope is shown with a generic UI.
   * Values:
   * 'arguments': Scope contains method arguments.
   * 'locals': Scope contains local variables.
   * 'registers': Scope contains registers. Only a single `registers` scope
   * should be returned from a `scopes` request.
   * 'returnValue': Scope contains a return value. This may represent a value
   * about to be returned, or a value just returned from a function call, for example.
   */
  presentationHint?: 'arguments' | 'locals' | 'registers' | 'returnValue' | string;

There is a bit of looseness in that this doesn't say where the return value is coming from. In cases like I mentioned above, there may be a return value prepared on the current call frame. In other cases (e.g. what Python does today) they may want to display the value returned from a function the user just stepped out of. Possibly both: the scope name could be used to differentiate.