[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Jim Ingham via Phabricator via cfe-commits]

jingham added a comment.

Yes definitely use names for your breakpoints.  It makes them easier to handle.  Note starting a month or two ago you can set names to not get deleted except by an explicit gesture.  That hasn't shown up in releases yet, but once you can rely on its being there, you can set the names to not get auto-deleted or disabled, and then if somebody does:

(lldb) break disable

not thinking they are affecting your utility, you won't get messed up by this.



================
Comment at: utils/clangdiag.py:87
+def disable(debugger):
+    global target
+    global diagtool
----------------
clayborg wrote:
> remove and use:
> ```
> target = debugger.GetSelectedTarget()
> ```
See my comment.  Don't use selected targets, use the command form that takes an SBExecutionContext.  That's been around for more than a couple of years now, so it's pretty safe to use.


================
Comment at: utils/clangdiag.py:91-92
+    # Remove all diag breakpoints.
+    for bp in Breakpoints:
+        target.BreakpointDelete(bp.GetID())
+    target = None
----------------
clayborg wrote:
> Another way to do this would be to give the breakpoints you create using "target.BreakpointCreateXXX" to have a name when you create them:
> 
> ```
> bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
> bp.AddName("llvm::Diagnostic")
> ```
> 
> Then here you can iterate over all breakpoints in the target:
> 
> ```
> for bp in target.breakpoint_iter():
>   if bp.MatchesName("llvm::Diagnostic"):
>     target.BreakpointDelete(bp.GetID())
> ```
> 
> Then you don't need a global list?
> 
If you name them you can just do:

target.BreakpointDeleteByName("llvm::Diagnostic")

That's even simpler.


https://reviews.llvm.org/D36347