[Lldb-commits] [lldb] lldb-dap: Stop using replicated variable ids (PR #124232)

[Anthony Eid via lldb-commits]

Anthony-Eid wrote:

@da-viper Thank you for your feedback! To address some of it

> I am not too happy with the SwitchFrame and ReadyFrame as it implies that we have a state within the stopped state.
I think it would make sense to unify these functions and see if we can get rid of `SwitchFrame`. 

> It may not work if we are changing scopes in a stopped state (have not tested this patch locally) from https://github.com/llvm/llvm-project/issues/147105.

This is one of the bugs I'm aiming to fix with this PR, specially because it's extremely noticeable in Zed's debugger.  Zed caches most of the requests it makes, and because lldb-dap reuses scope/variable references it's only able to get correct scopes/variables for the first frame it requests from. 

After running lldb-dap with my current branch the issue is fixed. I think it's because the variables request handler gets the correct scope from `GetTopLevelScope` (This calls `SwitchFrame` under the hood) and `GetScopeKind` correctly now.

 > It seems we are storing redundant information. For example to get the Global scope we have to SwitchFrame then use the public Variable:global. We could drop some of the current abstraction that may no longer apply like the Variables::global , local ...
>
> we can either have separate maps for variables and scope. or merge them together.

I'll see if I can implement this. I might need some help because I've been primarily programming in Rust for a while now and am quite rusty when it comes to C++ 😀


https://github.com/llvm/llvm-project/pull/124232