[Lldb-commits] [lldb] [lldb-dap] Implement command directives (PR #74808)

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 11 12:08:41 PST 2023


clayborg wrote:

> @clayborg , I actually insist on aborting on failure for this feature. You mention:
> 
> > It would be nice if the RunLLDBCommands function could take an extra bool &fatal_error that clients could check after running when needed to decide what to do if this is set to true due to any ! prefixed commands failing.
> 
> That would imply that some commands would have a specific behavior upon a failed ! command, and other commands would have a different behavior. That makes this feature a bit hard to use by actual users, as it wouldn't have a consistent behavior, unlike ? commands.
> 
> What I wanted to achieve with ! was to have a way for users to tell the lldb-dap: hey, this command is critical for my flow, abort if it fails. Having a mild behavior instead of aborting kind of defeats the purpose of this feature.

Yeah but does aborting tell the client anything useful? Lets say we are in the middle of doing an `attach` packet and an attach command fails, what do we see in the IDE? Probably something like "DAP crashed" and the user is going to need to know that they should check the debug console? I would much rather have the `attach` packet return an erorr that says "error: one of your required 'attachCommands' failed, check the debug console for details", or during `launch` seeing "error: one of your required 'launchCommands' failed, check the debug console for details".

If you look at how `request_attach` and `request_lauch` are configured, they do most of the `g_dap.RunXXXCommands` and the both packets allow us to return an error that would make it much more clear what went wrong.

There are a few other areas that run commands like `SendTerminatedEvent()` that calls `g_dap.RunTerminateCommands();` and `SendThreadStoppedEvent()` that can call `g_dap.RunStopCommands();`, but other than that they are mostly in `launch` or `attach`. 

So I don't see how just crashing the DAP helps in any way when we can easily return a very clear error to either `launch` or `attach` and the user will know exactly what went wrong. 

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


More information about the lldb-commits mailing list