Re: [PATCH 2/2] Add return value to DAP scope

["Kévin Le Gouguec" <legouguec@adacore.com>]

Tom Tromey <tromey@adacore.com> writes:

> diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
> index ff553259ea9..f3bbe4fc614 100644
> --- a/gdb/python/lib/gdb/dap/scopes.py
> +++ b/gdb/python/lib/gdb/dap/scopes.py
> @@ -25,17 +25,32 @@ from .varref import BaseReference
>  frame_to_scope = {}
>  
>  
> +# If the most recent stop was due to a 'finish', and the return value
> +# could be computed, then this holds that value.  Otherwise it holds
> +# None.
> +_last_return_value = None
> +
> +
>  # When the inferior is re-started, we erase all scope references.  See
>  # the section "Lifetime of Objects References" in the spec.
>  @in_gdb_thread
>  def clear_scopes(event):
>      global frame_to_scope
>      frame_to_scope = {}
> +    global _last_return_value
> +    _last_return_value = None

[… some lines elided …]
> +    def fetch_one_child(self, idx):
> +        global _last_return_value
> +        if _last_return_value is not None:
> +            if idx == 0:
> +                return ("(return)", _last_return_value)
> +            idx = idx - 1
> +        return super().fetch_one_child(idx)

If scopes are cleared whenever the inferior is resumed (my paraphrasing
of § Lifetime of Objects References), can this method be called after
_last_return_value has been reset to None?

Wondering if the outer 'if' is necessary, since we only create a
_FinishScopeReference when _last_return_value is not None:

> @@ -120,10 +158,13 @@ def scopes(*, frameId: int, **extra):
>          args = tuple(frame.frame_args() or ())
>          if args:
>              scopes.append(_ScopeReference("Arguments", "arguments", frame, args))
> +        has_return_value = frameId == 0 and _last_return_value is not None
>          # Make sure to handle the None case as well as the empty
>          # iterator case.
>          locs = tuple(frame.frame_locals() or ())
> -        if locs:
> +        if has_return_value:
> +            scopes.append(_FinishScopeReference("Locals", "locals", frame, locs))
> +        elif locs:
>              scopes.append(_ScopeReference("Locals", "locals", frame, locs))
>          scopes.append(_RegisterReference("Registers", frame))
>          frame_to_scope[frameId] = scopes

Tangentially, wondering if capturing _last_return_value as a
_FinishScopeReference.__init__ parameter would make sense.

It may well be that I don't completely understand the lifetime of the
objects involved here.  At any rate:

Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>