[PATCH] D36347: New lldb python module for adding diagnostic breakpoints
Don Hinton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 26 10:29:46 PDT 2017
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:
> 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
+ getDiagtool(exe_ctx.GetTarget(), args.path)
> should probably verify that this 'diagtool' with:
> elif args.subcommands == 'diagtool':
> and add an else that creates an error:
> 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.
More information about the cfe-commits