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

2017-10-27 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316773: New lldb python module for managing diagnostic breakpoints (authored by dhinton). Repository: rL LLVM https://reviews.llvm.org/D36347 Files: cfe/trunk/utils/clangdiag.py Index:

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

2017-10-27 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120629. hintonda added a comment. - Remove whitespace. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === --- /dev/null +++ utils/clangdiag.py @@ -0,0

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

2017-10-27 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120628. hintonda added a comment. - Add support for individual DiagID's, and print out number of breakpoints added. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py

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

2017-10-26 Thread Zachary Turner via cfe-commits
looks good! Feel free to commit whenever, I'd definitely recommend posting a PSA on cfe-dev@ (after you commit) so that people know about it. You might also get some useful ideas for improvements that way too. On Thu, Oct 26, 2017 at 9:52 PM Don Hinton wrote: > On Thu, Oct

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

2017-10-26 Thread Don Hinton via cfe-commits
On Thu, Oct 26, 2017 at 5:44 PM, Zachary Turner wrote: > > > On Thu, Oct 26, 2017 at 3:18 PM Don Hinton wrote: > >> On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner >> wrote: >> >>> Seems fine, it would be nice if the workflow could be

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120538. hintonda added a comment. - Add ability to add breakpoints matching -W warnings. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === ---

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

2017-10-26 Thread Zachary Turner via cfe-commits
On Thu, Oct 26, 2017 at 3:18 PM Don Hinton wrote: > On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner > wrote: > >> Seems fine, it would be nice if the workflow could be improved a little >> bit so that all you have to do is say `clangdiag break >>

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120518. hintonda added a comment. - Maintain process id map for diagtool. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === --- /dev/null +++

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

2017-10-26 Thread Don Hinton via cfe-commits
On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner wrote: > Seems fine, it would be nice if the workflow could be improved a little > bit so that all you have to do is say `clangdiag break > —error=“-Wcovered-switch”` or something . I think that gives the most > intuitive usage

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

2017-10-26 Thread Don Hinton via cfe-commits
On Thu, Oct 26, 2017 at 3:00 PM, Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added a comment. > > Each lldb.SBValue has accessors for the stuff in an execution context: > > `` > > lldb::SBTarget GetTarget(); > lldb::SBProcess GetProcess(); > lldb::SBThread

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

2017-10-26 Thread Greg Clayton via Phabricator via cfe-commits
clayborg added a comment. Each lldb.SBValue has accessors for the stuff in an execution context: `` lldb::SBTarget GetTarget(); lldb::SBProcess GetProcess(); lldb::SBThread GetThread(); lldb::SBFrame GetFrame(); You could keep a global map of process ID to diagtool if you want?

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

2017-10-26 Thread Zachary Turner via cfe-commits
Seems fine, it would be nice if the workflow could be improved a little bit so that all you have to do is say `clangdiag break —error=“-Wcovered-switch”` or something . I think that gives the most intuitive usage for people, even it’s a bit harder to implement. I also think user shouldn’t really

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Is there a way to associate a particular diagtool variable to an exe_ctx? https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120492. hintonda added a comment. - Remove debugging print statement, and enhance help message. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === ---

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. In https://reviews.llvm.org/D36347#908186, @zturner wrote: > Do I understand correctly that this will insert breakpoints on *all* clang > diagnostics? That's not necessarily bad, but I was under the impression > originally that it would let you pick the diagnostics

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

2017-10-26 Thread Greg Clayton via Phabricator via cfe-commits
clayborg added a comment. Looks good. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-10-26 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120459. hintonda added a comment. - Enhance diagtool option to check, reset, print out current value. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py

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

2017-10-26 Thread Greg Clayton via Phabricator via cfe-commits
clayborg accepted this revision. clayborg added inline comments. Comment at: utils/clangdiag.py:117 +disable(exe_ctx) +else: +getDiagtool(exe_ctx.GetTarget(), args.path[0]) hintonda wrote: > clayborg wrote: > > should probably verify that

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

2017-10-26 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. Do I understand correctly that this will insert breakpoints on *all* clang diagnostics? That's not necessarily bad, but I was under the impression originally that it would let you pick the diagnostics you wanted to insert breakpoints on. Also, What is the workflow

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

2017-10-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120360. hintonda added a comment. - Add diagtool option used to set arbitrary diagtool path. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py === ---

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

2017-10-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda added inline comments. Comment at: utils/clangdiag.py:83 +# Remove all diag breakpoints. +bkpts = lldb.SBBreakpointList(target) +target.FindBreakpointsByName("clang::Diagnostic", bkpts) Can't use iterator because it gets invalidated and not

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

2017-10-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 120340. hintonda edited the summary of this revision. hintonda added a comment. Herald added a subscriber: ilya-biryukov. - Addressed comments. https://reviews.llvm.org/D36347 Files: utils/clangdiag.py Index: utils/clangdiag.py

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

2017-10-25 Thread Jim Ingham via Phabricator via cfe-commits
jingham added a comment. Note, BTW, Enrico also added a form of the command add that allows you to use a Python class to wrap the callable. That was in early 2015 so it's probably safe to use as well by now. That's even more convenient, and obviates the need for globals at all. One instance

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

2017-10-25 Thread 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.

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

2017-10-25 Thread 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

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

2017-10-25 Thread Jim Ingham via Phabricator via cfe-commits
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Use the form of the command that gets an SBExecutionContext, then you can avoid having to cache the target at all. Comment at: utils/clangdiag.py:98-100 +def

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

2017-10-25 Thread Don Hinton via Phabricator via cfe-commits
hintonda added a comment. Thanks for the quick feedback. I'll make all you suggested changes, but need to think a little more about `diagtool`. Comment at: utils/clangdiag.py:75-78 +diagtool = os.path.join(exe.GetDirectory(), 'diagtool') +if not

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

2017-10-25 Thread Greg Clayton via Phabricator via cfe-commits
clayborg added a comment. Looks good. A little bit of cleanup. Let me know what you think of the comments. Comment at: utils/clangdiag.py:17 +import os +from subprocess import check_output as qx; + I would rather just do: ``` import subprocess ``` Then later