[Lldb-commits] [lldb] [lldb-dap] Refactor request handlers (NFC) (PR #128262)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 24 07:43:38 PST 2025


JDevlieghere wrote:

That's a good question. The non-constness of the operator was an oversight, I meant for the classes to be stateless and mimic the current implementation. Adding he `const` is trivial and I can do that before migrating more requests, but that's pointless if we want to make them stateful in the future. 

I'm looking at the request handlers trying to answer these two questions:

1. _Do the request handlers benefit from being stateful?_ That's not how they're implemented currently so it's hard to tell. We might be able to hoist some common logic out of the `operator` at the beginning (e.g. `FillResponse`) and the end (e.g. `dap.SendJSON`). But the latter will already go away with @ashgti's PR. 
2. _Should the request know they're interrupted before they complete?_ I see two potential benefits. If a request handlers relies on more than one SB API call, and it gets interrupted, we can avoid making more API calls and have to rely on it supporting interruption within LLDB. The second one is that maybe we can avoid building a response JSON object if we don't have to, but I think this one is less compelling because although that's most of the code in the requests handlers, I think this is pretty fast. Doing both of those things I've described here would increase complexity and I'm not sure if it's worth it. 
 
My gut feeling is that (2.1) is the most compelling, with (1) being a nice to have and (2.2) unlikely to outweigh the added complexity. In other words, I'm (weakly) in favor of making the handlers stateful and instantiating a new one for every request. But I'd love to hear what @ashgti thinks as he's the one working on the interruption support. 

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


More information about the lldb-commits mailing list