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

[Greg Clayton via Phabricator via cfe-commits]

clayborg added inline comments.


================
Comment at: utils/clangdiag.py:62
+
+def enable(debugger, args):
+    # Always disable existing breakpoints
----------------
Pass exe_ctx or target into this instead of the debugger (see Jim's comment on execution contexts below.


================
Comment at: utils/clangdiag.py:78
+        print('diagtool not found along side %s' % exe)
+        return
+
----------------
No need. just a suggestion. Is this information available in the main executable as any kind of debug info? If so you can read it from there. If it is always in a stand alone separate file, then what you have is ok.


================
Comment at: utils/clangdiag.py:87
+def disable(debugger):
+    global target
+    global diagtool
----------------
jingham wrote:
> 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.
Just pass the target or exe_ctx into this function then instead of "debugger".


================
Comment at: utils/clangdiag.py:92
+    for bp in Breakpoints:
+        target.BreakpointDelete(bp.GetID())
+    target = None
----------------
nice!


================
Comment at: utils/clangdiag.py:100
+    # Use the Shell Lexer to properly parse up command options just like a
+    # shell would
+    command_args = shlex.split(command)
----------------
Great idea. Forgot about the execution context variant. The "exe_ctx" is a lldb.SBExecutionContext object. You get your target using:

```
target = exe_ctx.GetTarget()
```



https://reviews.llvm.org/D36347