[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

[Callum Macmillan via Phabricator via lldb-commits]

cimacmillan marked 6 inline comments as done.
cimacmillan added inline comments.


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2188
+
+static std::optional<std::string> set_data_breakpoint(const llvm::json::Object *breakpoint) {
+  std::string breakpoint_id = GetString(breakpoint, "id").str();
----------------
clayborg wrote:
> This function should probably be able to return an error for each watchpoint if anything fails so this can be reported back as a description or error response in the "setDataBreakpoints" response? We shouldn't be printing to the console for errors in the "breakpoint" object arguments or if the watchpoint actually fails to resolve (was in a register). An error message should be returned somehow if this is possible?
Thanks for your feedback, working through comments now. About this and 

> Is there a way to return an error for a specific data breakpoint instead of us printing to the console?

We can return whether the breakpoint is verified, which displays a greyed dot similar to when a line breakpoint can't be set. There doesn't seem to be a way to display the error in VS code with the message. I think the logging would be useful in that case, but I can also include it in the message response in case this is added to VS code.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140630/new/

https://reviews.llvm.org/D140630