[Lldb-commits] [lldb] [lldb] refactor watchpoint functionality (PR #159807)

[via lldb-commits]

================
@@ -621,63 +621,72 @@ void Thread::WillStop() {
 }
 
 bool Thread::SetupToStepOverBreakpointIfNeeded(RunDirection direction) {
-  if (GetResumeState() != eStateSuspended) {
-    // First check whether this thread is going to "actually" resume at all.
-    // For instance, if we're stepping from one level to the next of an
-    // virtual inlined call stack, we just change the inlined call stack index
-    // without actually running this thread.  In that case, for this thread we
-    // shouldn't push a step over breakpoint plan or do that work.
-    if (GetCurrentPlan()->IsVirtualStep())
-      return false;
+  if (GetResumeState() == eStateSuspended)
----------------
dlav-sc wrote:

I refactored this part as well because I plan to implement the logic for adding a `WatchpointThreadPlan` to the thread plan stack within this function. As I mentioned above, the logic for software watchpoints is planned to be implemented using a special `ThreadPlan`.

Essentially, the entire refactoring of this function boils down to adding a couple of early return statements. Personally, I strongly dislike cascades of nested `if/else` blocks. I understand that several years ago it was apparently fashionable to write functions with a single `return` statement, so I'm lenient towards that practice. However, early returns are much easier to read, and since I'm about to add new logic to this function anyway, I figured why not tidy it up first.

Although, if you think it's better to implement this part together with the logic for adding the thread plan, I wouldn't be opposed. We can leave this for the next patch.

https://github.com/llvm/llvm-project/pull/159807