jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Resetting the hit counts on rerun is a more useful behavior, so this is all
fine. But the Target is the one that does all the breakpoint management, so I
think the resetting should go through the Target, not the Process. And we
generally delegate "do on all breakpoints" operations to the BreakpointList, so
it would be more consistent with the current structure to go
Process::WillLaunch -> Target::ResetHitCounts -> BreakpointList::ResetHitCounts.
================
Comment at: lldb/source/Target/Process.cpp:2763-2766
+static void ResetHitCounts(Process &Proc) {
+ for (const auto &BP : Proc.GetTarget().GetBreakpointList().Breakpoints())
+ BP->ResetHitCount();
+}
----------------
fdeazeve wrote:
> JDevlieghere wrote:
> > Can this be a private member so we don't have to pass in `*this`?
> No strong preferences on my part, but I had made it a free function because
> it can be implemented in terms of the public behaviour, i.e. it doesn't rely
> on implementation details.
Resetting all the hit counts in a BreakpointList seems to me a job of the
BreakpointList. Also, while DoWillLaunch/Attach is where this action belongs,
which is how the Process gets involved, the Target is the one that manages the
breakpoints in general. So I think it would be better to have the Target do
ResetHitCounts, and these Process calls should just dial up it's Target.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133858/new/
https://reviews.llvm.org/D133858
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
