hintonda added a comment.
Thanks for the feedback (addressed below).
btw, where should this module go?
Since it's intended for clang, and clang based tool, developers, I put it in
`clang/utils`, but Zackery suggested `lldb/examples/python` might be a better
place. Please let me know if anyone has a preference.
================
Comment at: utils/clangdiag.py:66
+def setDiagBreakpoint(frame, bp_loc, dict):
+ id = frame.FindVariable("DiagID").GetValue()
+ if id is None:
----------------
clayborg wrote:
> Is there only ever one of these? If so this is good.
Currently, DiagnosticsEngine::Report only has two signatures, and both contain
an `unsigned DiagID` parameter, so this is just a sanity check to make sure we
are in the right place.
================
Comment at: utils/clangdiag.py:117
+ disable(exe_ctx)
+ else:
+ getDiagtool(exe_ctx.GetTarget(), args.path[0])
----------------
clayborg wrote:
> should probably verify that this 'diagtool' with:
>
> ```
> elif args.subcommands == 'diagtool':
> ```
> and add an else that creates an error:
> ```
> else:
> result.SetError("invalid subcommand '%s'" % (args.subcommands))
> ```
This is already handled by `argparser`, which will raise an exception if the
first parameter isn't one of (enable, disable, daigtool). That's the benefit
of using `subparsers`. So, by the time you get to this point, it must be
"diagtool".
However, I think I should probably verify the path past actually exists. Plus,
I think I could add a better exception to alert the user how to fix the problem
if diagtool couldn't be found in the callback. Suggestions welcome.
https://reviews.llvm.org/D36347
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
