> On Oct 29, 2015, at 3:34 AM, Mohit Bhakkad <mohit.bhak...@gmail.com> wrote: > > mohit.bhakkad added a comment. > > In http://reviews.llvm.org/D13296#265608, @jingham wrote: > >> This change alters the timing for the handling of ignore counts for >> watchpoints. The original implementation (and the way ignore counts work >> for breakpoints) is that the breakpoint's ignore count gets checked during >> the "synchronous" phase of breakpoint stop evaluation. So it happens when >> the private stop event is received, and if the ignore count is not satisfied >> then a public event is not generated. >> >> Your version will move the decision making to the async ShouldStop, i.e. >> when an event is getting pulled from the public event queue. If these were >> breakpoints it would mean that the synchronous callbacks would still fire >> even if they fail their ignore count test. Watchpoints don't currently have >> synchronous callbacks - and I'm not sure we would ever need to support that. >> If you moved the adjustment code you do in PerformAction to the synchronous >> callback (Watchpoint::ShouldStop) that would keep breakpoints & watchpoints >> behaving symmetrically. It's worth trying that, I am pretty sure that will >> be a safe place to do your little dance. But if it isn't, then I don't mind >> having it in PerformAction provided you put a note to that effect somewhere >> obvious in Watchpoint.h. > > > Hi @jingham, I tried to move adjustment code to ShouldStopSynchronous before > we call ShouldStop, but its not getting stepped to next PC. Looks like > threadplan are not active in that phase of code. Could you suggest anything > wrong I may be doing or should I resubmit this patch by putting condition > code before we print anything.
I think it is fine to do that latter. The watchpoint code really needs an overhaul anyway, so taking the easy route here seems fine. Just leave a comment somewhere explaining the behavior difference so when we get around to fixing it up we'll remember that it works this way. Jim > >> Note, if you end up going with the current patch, isn't quite right, >> however. You need to move the check for the watchpoint ignore count up >> before printing out the old & new values. An ignored watchpoint shouldn't >> print anything, it should be as if it never stopped. Where you have the >> check, the old & new values will get printed even if the watchpoint is >> ignored. > > > > Repository: > rL LLVM > > http://reviews.llvm.org/D13296 > > > _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
