[Lldb-commits] [PATCH] D88129: Add "break delete --disabled" to delete all currently disabled breakpoints

[Jim Ingham via Phabricator via lldb-commits]

jingham marked an inline comment as done.
jingham added a comment.

I agree that having --disable flip the meaning of the listed breakpoints, but I think its the most useful meaning by far.



================
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:1475
     Target &target = GetSelectedOrDummyTarget(m_options.m_use_dummy);
-
+    result.Clear();
+    
----------------
kastiglione wrote:
> why is this added?
I depend on the result being Success, but not all code paths actually use it.  So I want to make sure it's in the Success state when I get started.


================
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:1480
 
-    const BreakpointList &breakpoints = target.GetBreakpointList();
+    BreakpointList &breakpoints = target.GetBreakpointList();
 
----------------
kastiglione wrote:
> does iterating over `breakpoints.Breakpoints()` require this to be non-const?
Right.


================
Comment at: lldb/source/Commands/Options.td:232
+    Desc<"Delete all breakpoints which are currently disabled.  When using the disabled option "
+    "any breakpoints listed on the command line are EXCLUDED from deletion.">;
 }
----------------
kastiglione wrote:
> To me, it's counter intuitive that `break delete --disabled 1` will not delete bp 1.
The combination:

(lldb) break delete --disabled 1

could either mean 

1) delete all breakpoints that are disabled AND breakpoint 1
2) delete all breakpoints that are disabled EXCEPT breakpoint 1
3) an error

Of those interpretations, 1 and 3 don't seem very useful, but 2 does.  

This is particularly handy when you specify a breakpoint name, not a breakpoint.  Just make breakpoints you don't want deleted DoNotDelete, then you can easily protect all those breakpoints.  

Note, your workaround would only be useful in this case if all the breakpoints named DoNotDelete are currently disabled.  Otherwise you would have to remember which of the DoNotDelete breakpoints were disabled, enable them all, do the `delete --disabled` then  only re-disable those that were originally disabled.  Whereas if you can pass an exclude list you can just protect those breakpoints unconditionally regardless of their state.

So while I agree this is a little odd, it's actually the only option that really makes sense, it's easy to document, and I don't think it's likely to cause mistakes. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88129