================
@@ -940,12 +940,44 @@ bool Thread::ShouldStop(Event *event_ptr) {
if (should_stop)
current_plan->WillStop();
- if (current_plan->ShouldAutoContinue(event_ptr)) {
+ const bool auto_continue =
+ current_plan->ShouldAutoContinue(event_ptr);
+ if (auto_continue) {
override_stop = true;
LLDB_LOGF(log, "Plan %s auto-continue: true.",
current_plan->GetName());
}
+ // A plan that auto-continues has already settled that this stop will
----------------
jimingham wrote:
The change looks fine, but the comment is over-verbose and defensive. It's not
necessary to explain the circumstance in which you found the bug, that's not
going to help future readers of the code.
Sometimes if something is tricky it's useful to say "this is tricky" but in
this case it really isn't. You are just tightening up the contract from
ThreadPlan.h:
```
/// Returns whether this thread plan overrides the `ShouldStop` of
/// subsequently processed plans.
///
/// When processing the thread plan stack, this function gives plans the
/// ability to continue - even when subsequent plans return true from
/// `ShouldStop`. \see Thread::ShouldStop
virtual bool ShouldAutoContinue(Event *event_ptr) { return false; }
```
so that not only does it override, but it does not consult the ShouldStop of
threads that say they need to auto-continue. So the appropriate way to explain
this is to say in the contract that it will short-circuit ShouldStop. And then
say here that you are obeying that contract, if anything.
I'm not even sure it's worth explaining why the old way caused problems. In
hindsight, asking a thread plan a question you plan to discard the answer of
didn't really make all that much sense. So the new contract stands on its own.
https://github.com/llvm/llvm-project/pull/215522
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
