[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

Good call on calling ShouldAutoContinue before Pop.



================
Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop
----------------
jingham wrote:
> kastiglione wrote:
> > Happy to iterate on this. I wanted to take the opportunity to explain this, since by name it may seem unclear how exactly it relates to `ShouldStop`.
> The use of "previous plans" is a little confusing, since it not clear "previous to what".   To me the obvious meaning is plans previously dealt with in this negotiation, which is not what you mean, and then sounds really odd when you hit the "subsequently".  I don't think you need the "previous" at all.  You could just say "subsequently processed plans".
Thanks I'll update it.

I agree that "previous" can be ambiguous, especially in the context of `Thread::ShouldStop`. What do you think of renaming `GetPreviousPlan` to `GetParentPlan` (and related variables and documentation too)? I can do this in a follow up if you agree.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97076/new/

https://reviews.llvm.org/D97076