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.

[theIDinside]

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?

After having processed this further, these are my thoughts:

Scope comes with problems but mainly, the biggest problem is: The client may or may not decide to show the scope automatically, after a stepOut has completed and for the debug adapter maintainers, we are out of luck, we have no way to influence that it lands in a scope that's displayed (let's use VSCode as an example since it's the biggest client in the wild) - unless we artificially make it so it's in the first scope, in the scope list which VSCode generally tends to display automatically in my experience.

This is counter intuitive. After having thought about it, I actually think my initial example is superior (@tromey suggested an OutputEvent, and even though that's not the same as what I am suggesting, it has some of the same benefits, though not all).

But I have come to realize that adding a new reason is what is not good with my initial proposal - it should just be one of the existing reasons.

The reason for that, is adding just an optional field to StoppedEvent called result we can actually stuff both return values and data breakpoint values there. And this remains backwards compatible.

interface StoppedEvent {
  /**
   * A value that is related to this stopped event. Used, not exclusively, to signal return values
   * or data breakpoint values.
   */
  result?: Variable;

  /// ...
}

I also don't see how this could not work in the scenario that @connor4312 mentions - take for instance a C++ exception, and the debugger & adapter has catch exceptions (I use GDB terminology throughout here) - it will stop in a catch block, and in that case, the exception value could be provided (although, obviously, it will also exist on the stack frame - but the point still remains the same). Sticking an optional field on every StackFrame I think is counter intuitive as well and the problems with it, has been pointed out by @gregg-miskelly

The thing is - we want to be able to represent a value with a stop. I don't know how Go and Zig does with their defer statements, but I don't see how the optional on the StoppedEvent doesn't work there either; if the user doesn't have a breakpoint set there, or doesn't stop right after the code that produces some value, the user will never be able to see the value anyhow.

This will also work very well with logging purposes (which sticking on StackFrame or using Scopes, certainly will not) as a debug adapter can say "continuously step in and out of these functions and log the value associated with the StoppedEvents we come across", for instance.

Stop events are also always processed and must be supported (although a client doesn't have to support the optional result property) so what @puremourning brings up, doesn't apply here.

Additionally, this could potentially open up for debuggers and debug adapters, that step over a return statement (in whatever programming language we're talking about) can have a result variable associated with the StoppedEvent without having to anything hacky - it would be well specified and defined. And although it's subjective on my behalf, it is very simple and very intuitive what it means. It means it's a stop event, with an associated value, it can represent many things.

[theIDinside]

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.

If I had any say, I would vote a hard no on the scope idea. I guess we would have to reach out to more maintainers of debug adapters, but I don't think scope works as well as stopped event does. It doesn't compose well or extend well.

The reason why StoppedEvent would work so well, is because it has a very well defined set of reasons, which means that interpreting the result property is already a built-in mechanism, for free.

[puremourning]

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?

After having processed this further, these are my thoughts:

Scope comes with problems but mainly, the biggest problem is: The client may or may not decide to show the scope automatically, after a stepOut has completed and for the debug adapter maintainers, we are out of luck, we have no way to influence that it lands in a scope that's displayed (let's use VSCode as an example since it's the biggest client in the wild) - unless we artificially make it so it's in the first scope, in the scope list which VSCode generally tends to display automatically in my experience.

This is counter intuitive. After having thought about it, I actually think my initial example is superior (@tromey suggested an OutputEvent, and even though that's not the same as what I am suggesting, it has some of the same benefits, though not all).

But I have come to realize that adding a new reason is what is not good with my initial proposal - it should just be one of the existing reasons.

The reason for that, is adding just an optional field to StoppedEvent called result we can actually stuff both return values and data breakpoint values there. And this remains backwards compatible.

interface StoppedEvent {

  /**

   * A value that is related to this stopped event. Used, not exclusively, to signal return values

   * or data breakpoint values.

   */

  result?: Variable;



  /// ...

}

I also don't see how this could not work in the scenario that @connor4312 mentions - take for instance a C++ exception, and the debugger & adapter has catch exceptions (I use GDB terminology throughout here) - it will stop in a catch block, and in that case, the exception value could be provided (although, obviously, it will also exist on the stack frame - but the point still remains the same). Sticking an optional field on every StackFrame I think is counter intuitive as well and the problems with it, has been pointed out by @gregg-miskelly

The thing is - we want to be able to represent a value with a stop. I don't know how Go and Zig does with their defer statements, but I don't see how the optional on the StoppedEvent doesn't work there either; if the user doesn't have a breakpoint set there, or doesn't stop right after the code that produces some value, the user will never be able to see the value anyhow.

This will also work very well with logging purposes (which sticking on StackFrame or using Scopes, certainly will not) as a debug adapter can say "continuously step in and out of these functions and log the value associated with the StoppedEvents we come across", for instance.

Stop events are also always processed and must be supported (although a client doesn't have to support the optional result property) so what @puremourning brings up, doesn't apply here.

Additionally, this could potentially open up for debuggers and debug adapters, that step over a return statement (in whatever programming language we're talking about) can have a result variable associated with the StoppedEvent without having to anything hacky - it would be well specified and defined. And although it's subjective on my behalf, it is very simple and very intuitive what it means. It means it's a stop event, with an associated value, it can represent many things.

I'm not sure what you're referring to that I said, but clients still don't know where or how to display this Variable.

What would you do if you were a client/UI implemented? Where would you surface this data?

[theIDinside]

I'm not sure what you're referring to that I said, but clients still don't know where or how to display this Variable.

What would you do if you were a client/UI implemented? Where would you surface this data?

I was referring to you mentioning that OutputEvent would be bad and how that wouldn't fly and how I agreed that was the problem with OutputEvent, because an OutputEvent is by it's very nature asynchronous (and it does not stop the state machine which is an UI/Debugger session in the same fashion a StoppedEvent does since it is the defacto way to trigger an update chain, i.e. [threads, stacktrace, scopes, variables_1 ... variables_n])

However, representing it in the UI would be the debug adapter's responsibility - it would also be trivial, since a stop event always precedes an update chain, it would be the DA's responsibility to surface the data. If you want to use a scope, use a scope. If you want to use, for instance, VSCode's exception UI - if you want to give room for VSCode to actually develop new UI widgets, StoppedEvent would be the go to solution as it's composable, extendable by default.

Locking returnValue into Scopes, means locking every single other "stop-associated value" as well, where you have to keep adding it to scopes - and if, for some reason, the debug adapter wants to automate something, or build some fancy tool, using the scopes approach would now require it to report the stop event to the client, and then read the update chain results.

I don't see any composability arguments in Scopes favor. Could an argument be made that perhaps it's easier for VSCode to implement? Sure. But I would argue it comes at the cost of the protocol's quality and it would be another ad-hoc solution, instead of the more generic one.

[theIDinside]

Could a StoppedEvent value, be made to always be displayed in the Watch UI? So the same place that evaluateRequest goes?

I guess, this would marry the two worlds - any value seen in a StoppedEvent would be placed in the evaluateRequest UI, without adding additional complexity to Scopes - and it would also be a value produced immediately to the debug adapter, before having triggered a UI-update chain.

[connor4312]

Scope comes with problems but mainly, the biggest problem is: The client may or may not decide to show the scope automatically, after a stepOut has completed and for the debug adapter maintainers, we are out of luck, we have no way to influence that it lands in a scope that's displayed (let's use VSCode as an example since it's the biggest client in the wild) - unless we artificially make it so it's in the first scope, in the scope list which VSCode generally tends to display automatically in my experience.

In any case, whether we add a StoppedEvent.result, StackFrame.returnValue, or Scope.presentationHint, a client who wanted to display the return value nicely would need to be updated to be aware of the new data type. So, whether a client "may or may not decide" to show it in any specific way is still up to them. The only immediate difference is that the use of a Scope.presentationHint would show the return value in its own scope in existing clients without requiring changes, and they may later decide to display it specially.

The reason for that, is adding just an optional field to StoppedEvent called result we can actually stuff both return values and data breakpoint values there... the exception value could be provided.

Separate issue, but I think that data breakpoint values are better tracked in the Variable.presentationHint, since the variable will still live in its original Scope and we would not really want to relocate it and duplicate/lose information in the stopped event. For exceptions, it may be the case that both the return value and a caught exception are present in the same context, and a client may want to differentiate them in its display.

I don't know how Go and Zig does with their defer statements, but I don't see how the optional on the StoppedEvent doesn't work there either; if the user doesn't have a breakpoint set there, or doesn't stop right after the code that produces some value, the user will never be able to see the value anyhow.

Take the following Go code:

func c() int {
	return 3 // breakpoint here
}

func b() int {
	defer c()
	return 2
}

func a() int {
	defer b()
	return 1
}

Stopped in c, the call frames for both a() and b() will have associated return values. If we're adding a notation for return values into DAP, it would be nice to be able to represent those.

[puremourning]

My point about the stoped event is context. Debugger UI gets a stoped event with no context. I want to display the return value somewhere on the screen that has some meaning to the user such as inline with the function that was just called. From a stopped event I have no way to do that. Indeed that's true of scope and output event too, both of which are arguably worse, but at least there is a logical place to display the return value.

I could just stuff the stopped event 'return' value somewhere in the stack frame window. But that's not really intuitive either.

I'm not saying don't use a stopped event, I'm saying provide the UI with more context like a source range associated with where the rerun value came from or something like that.

Iirc it's valid to get multiple stopped events (one per thread eg) but the treadId is optional. So I just think that implementation in a client in practice should be considered while designing the API

[connor4312]

I think that case is covered by #343, which I'm anticipating taking up in the near future

[puremourning]

I think that case is covered by #343, which I'm anticipating taking up in the near future

Ah yes. Hadn't seen that. If we had that source/line info in Variable then I think this works well as a Stopped event argument.

[connor4312]

I'm still leaning toward use of a scope with presentationHint as the way to go here. It does all the things people have asked for:

• multiple frames with return values,
• allowance for the runtime to indicate return value computation may be Scope.expensive as Gregg mentioned
• "continuously step in and out of these functions and log the value associated with the StoppedEvents we come across" -- implementable, requires some legwork vs inline in the StoppedEvent, but still easy to implement

Are there UX scenarios folks see that it isn't satisfied by this case, or are satisfied poorly?

---

Re further up the thread:

StoppedEvent would be the go to solution as it's composable, extendable by default. Locking returnValue into Scopes, means locking every single other "stop-associated value" as well, where you have to keep adding it to scopes...

I'm not sure what you mean here, or that I get your point around lacking extensibility. A result?: Variable on the StoppedEvent is generic only insofar as it lacks specificity, requiring assumptions on the client to know what it is. In your examples, a return value versus an exception -- we could add further notation on it to say something like "if the stop reason is one of these, it's an exception, otherwise it's a return value." However, that restricts us to have exactly two types of data represented in the result (without adding separate stop reasons) and also doesn't handle cases like steping over a function that throws an exception and landing in a catch block.

In contrast, scope.presentationHint=returnValue describes exactly the data one should find there, and adding a new type like exceptionValue is not hard and is also explicit.

some fancy tool, using the scopes approach would now require it to report the stop event to the client, and then read the update chain results.

I don't think this is very fancy, a client reading the scopes for a stack frame is bread and butter of debugging.

[devarmenchik1408]

Как разработчик адаптера отладки, меня несколько раз спрашивали о возможности каким-то образом отображать возвращаемые значения, и даже несмотря на то, что это выполнимо с небольшими изменениями (фактически реализовано примерно так, как описано здесь, поэтому оно нестандартно). соответствует), было бы неплохо, если бы это было стандартизировано в протоколе.

Поскольку у нас есть stepOutзапрос, который явно «завершает» текущую функцию (на жаргоне GDB), reasonнеобходимо добавить новое значение, чтобы отразить, что это произошло, а не просто сообщить об этом step. По этой новой причине в Variableфайл также следует добавить необязательное поле типа StoppedEvent, в которое возвращается структурированное представление возвращаемого значения.

Я думаю, что явная stepOutпричина заключается в том, чтобы предпочесть, чем просто оставить его step, а затем включить необязательно valueили что-то в этом роде, StoppedEventпотому что это не так явно сигнализирует о намерении. Тем более, что stepостановки бывают разных форм (следующая строка, следующая инструкция, следующий оператор и т. д.).

Кроме того, добавление дополнительной stepOutпричины означает, что устаревшие адаптеры могут продолжать сигнализировать, stepесли они того пожелают, если они не желают или не хотят поддерживать функцию отображения возвращаемых значений после stepOutзапроса.
[
{ "objectID": "ID01", "title": "Chair", "color": "red", "categories": ["Furniture", "Outdoors"] },
{ "objectID": "ID02", "title": "Table", "color": "green", "categories": ["Furniture", "Outdoors"] },
{ "objectID": "ID03", "title": "Water bottle", "color": "blue", "categories": ["Outdoors"] },
{ "objectID": "ID04", "title": "Book", "color": "red", "categories": ["Books"] },
{ "objectID": "ID05", "title": "Headphones", "color": "green", "categories": ["Electronics"] },
{ "objectID": "ID06", "title": "Phone", "color": "blue", "categories": ["Electronics"] },
{ "objectID": "ID07", "title": "Television", "color": "red", "categories": ["Electronics"] },
{ "objectID": "ID08", "title": "Can", "color": "green", "categories": ["Cooking & Dining", "Kitchenware"] },
{ "objectID": "ID09", "title": "Bowl", "color": "blue", "categories": ["Cooking & Dining", "Kitchenware"] }
]

[tromey]

take for instance a C++ exception, and the debugger & adapter has catch exceptions (I use GDB terminology throughout here) - it will stop in a catch block, and in that case, the exception value could be provided (although, obviously, it will also exist on the stack frame - but the point still remains the same).

It's worth noting that a catch can be anonymous (like try {...} catch (int) { ...}), so the variable won't be visible -- but on some systems, gdb knows how to access the exception in flight and so it could still be displayed.

I personally tend to lean toward the new scope idea as well.

In the current implementation in gdb, I think I misunderstood the earlier discussion and so right now gdb just sticks a (return) variable into the locals scope.