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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to