[lldb][NFC] Move SBThread::ResumeNewPlan out of the header

[felipepiovezan]

This private method is only defined as a member class of SBThread so that it may be declared a friend of SBError and access a private constructor of SBError that takes a Status, which is an lldb_private object. In other words, the method does not use anything about the class is belongs to, so it has no business being a member method.

A subsequent patch will need to add a new argument to this class, a Process::StopLocker, which is also another lldb_private object. This is another strong suggestion that this method does not belong on the header file of a public API, even if at the private visibility level. We can't forward declare nested types like Process:StopLocker, so this problem is not easily solvable.

This commit moves the method out of the header and into an anon namespace of the cpp file. It is also made to return a Status instead, and the private SBError constructor is accessed by the SBThread methods that call it.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This private method is only defined as a member class of SBThread so that it may be declared a friend of SBError and access a private constructor of SBError that takes a Status, which is an lldb_private object. In other words, the method does not use anything about the class is belongs to, so it has no business being a member method.

A subsequent patch will need to add a new argument to this class, a Process::StopLocker, which is also another lldb_private object. This is another strong suggestion that this method does not belong on the header file of a public API, even if at the private visibility level. We can't forward declare nested types like Process:StopLocker, so this problem is not easily solvable.

This commit moves the method out of the header and into an anon namespace of the cpp file. It is also made to return a Status instead, and the private SBError constructor is accessed by the SBThread methods that call it.

---

Full diff: https://github.com/llvm/llvm-project/pull/151988.diff

2 Files Affected:

• (modified) lldb/include/lldb/API/SBThread.h (-3)
• (modified) lldb/source/API/SBThread.cpp (+7-17)

diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index f8ae627da5ace..e9fe5858d125e 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -251,9 +251,6 @@ class LLDB_API SBThread {
 
   void SetThread(const lldb::ThreadSP &lldb_object_sp);
 
-  SBError ResumeNewPlan(lldb_private::ExecutionContext &exe_ctx,
-                        lldb_private::ThreadPlan *new_plan);
-
   lldb::ThreadSP GetSP() const;
 
   lldb::ExecutionContextRefSP m_opaque_sp;
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index d9469fc1390d6..74bc66c4f16f1 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -495,21 +495,14 @@ bool SBThread::GetInfoItemByPathAsString(const char *path, SBStream &strm) {
   return success;
 }
 
-SBError SBThread::ResumeNewPlan(ExecutionContext &exe_ctx,
-                                ThreadPlan *new_plan) {
-  SBError sb_error;
-
+static Status ResumeNewPlan(ExecutionContext &exe_ctx, ThreadPlan *new_plan) {
   Process *process = exe_ctx.GetProcessPtr();
-  if (!process) {
-    sb_error = Status::FromErrorString("No process in SBThread::ResumeNewPlan");
-    return sb_error;
-  }
+  if (!process)
+    return Status::FromErrorString("No process in SBThread::ResumeNewPlan");
 
   Thread *thread = exe_ctx.GetThreadPtr();
-  if (!thread) {
-    sb_error = Status::FromErrorString("No thread in SBThread::ResumeNewPlan");
-    return sb_error;
-  }
+  if (!thread)
+    return Status::FromErrorString("No thread in SBThread::ResumeNewPlan");
 
   // User level plans should be Controlling Plans so they can be interrupted,
   // other plans executed, and then a "continue" will resume the plan.
@@ -522,11 +515,8 @@ SBError SBThread::ResumeNewPlan(ExecutionContext &exe_ctx,
   process->GetThreadList().SetSelectedThreadByID(thread->GetID());
 
   if (process->GetTarget().GetDebugger().GetAsyncExecution())
-    sb_error.ref() = process->Resume();
-  else
-    sb_error.ref() = process->ResumeSynchronous(nullptr);
-
-  return sb_error;
+    return process->Resume();
+  return process->ResumeSynchronous(nullptr);
 }
 
 void SBThread::StepOver(lldb::RunMode stop_other_threads) {

[jimingham]

Yeah, that's weird to be a method because it acts on a thread that isn't necessarily the thread that is managed by the SBThread calling the method. That does sound more like it should be a free function.