labath added a comment.

In D76968#1951707 <https://reviews.llvm.org/D76968#1951707>, @clayborg wrote:

> Looks good as long as with we have "llvm::Optional<llvm::StringRef> 
> request_path = {}" in the arguments for a function, that "{}" will turn into 
> llvm::None and not an empty StringRef? Unclear to me. As long as this is what 
> is happening and as long is this coding convention is used elsewhere in llvm 
> (using "{}" instead of "llvm::None") I am ok. Pavel?


The two expressions are equivalent here. We're currently using both styles in 
lldb. I have a slight preference for the explicit None version, but I don't 
think it's worth the trouble to standardize on one or the other.



================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1974
       // Add this breakpoint info to the response
       AppendBreakpoint(pair.second.bp, response_breakpoints);
     }
----------------
wallace wrote:
> labath wrote:
> > What about breakpoints in dynamically loaded shared libraries? Should you 
> > be remembering the original path somewhere so that one you can set it here 
> > too?
> This case is different, as function breakpoints are set by the IDE without 
> referring to any source path, as they are equivalent to making 'b 
> function_name'. Probably function breakpoints are not working correctly, so 
> I'll have to fix it any of these days using the expensive query I mentioned 
> above.
Ah, sorry. I misplaced this comment. It was meant to go above to line 417, 
where the `eBreakpointEventTypeLocationsAdded` event is handled. That code 
should be reached for "regular" file+line breakpoints in case a breakpoint gets 
new locations (e.g.) in response to a shared library being loaded.


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:421
             llvm::json::Object body;
             body.try_emplace("breakpoint", CreateBreakpoint(bp));
             body.try_emplace("reason", "changed");
----------------
The comment was supposed to be here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76968



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to