Address code review feedback

Author: JDevlieghere

lldb/include/lldb/API/SBLock.h

@@ -28,11 +28,13 @@ class LLDB_API SBLock {
   bool IsValid() const;
 
 private:
-  friend class SBTarget;
-
   SBLock() = default;
-  SBLock(std::recursive_mutex &mutex);
+
+  // Private constructor used by SBTarget to create the Target API lock.
+  // Requires a friend declaration.
   SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp);
+  friend class SBTarget;
+
   SBLock(const SBLock &rhs) = delete;
   const SBLock &operator=(const SBLock &rhs) = delete;
 

lldb/include/lldb/API/SBTarget.h

@@ -947,7 +947,7 @@ class LLDB_API SBTarget {
   lldb::SBTrace CreateTrace(SBError &error);
 
 #ifndef SWIG
-  lldb::SBLock GetAPILock() const;
+  lldb::SBLock AcquireAPILock() const;
 #endif
 
 protected:

lldb/include/lldb/Target/Target.h

@@ -1695,6 +1695,7 @@ class Target : public std::enable_shared_from_this<Target>,
 /// The private implementation backing SBLock.
 struct APILock {
   APILock(std::recursive_mutex &mutex) : lock(mutex) {}
+  virtual ~APILock() = default;
   std::lock_guard<std::recursive_mutex> lock;
 };
 
@@ -1703,6 +1704,7 @@ struct APILock {
 struct TargetAPILock : public APILock {
   TargetAPILock(std::recursive_mutex &mutex, lldb::TargetSP target_sp)
       : APILock(mutex), target_sp(target_sp) {}
+  ~TargetAPILock() override = default;
   lldb::TargetSP target_sp;
 };
 

lldb/source/API/SBLock.cpp

@@ -19,11 +19,6 @@ using namespace lldb_private;
 
 #ifndef SWIG
 
-SBLock::SBLock(std::recursive_mutex &mutex)
-    : m_opaque_up(std::make_unique<APILock>(mutex)) {
-  LLDB_INSTRUMENT_VA(this);
-}
-
 SBLock::SBLock(std::recursive_mutex &mutex, lldb::TargetSP target_sp)
     : m_opaque_up(std::make_unique<TargetAPILock>(mutex, target_sp)) {
   LLDB_INSTRUMENT_VA(this);

lldb/source/API/SBTarget.cpp

@@ -7,11 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBTarget.h"
-#include "lldb/API/SBLock.h"
-#include "lldb/Utility/Instrumentation.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/lldb-public.h"
-
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBEnvironment.h"
@@ -60,11 +55,14 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Instrumentation.h"
+#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/ValueObject/ValueObjectConstResult.h"
 #include "lldb/ValueObject/ValueObjectList.h"
 #include "lldb/ValueObject/ValueObjectVariable.h"
+#include "lldb/lldb-public.h"
 
 #include "Commands/CommandObjectBreakpoint.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -2437,7 +2435,7 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) {
 }
 
 #ifndef SWIG
-lldb::SBLock SBTarget::GetAPILock() const {
+lldb::SBLock SBTarget::AcquireAPILock() const {
   LLDB_INSTRUMENT_VA(this);
 
   if (TargetSP target_sp = GetSP())

lldb/unittests/API/SBLockTest.cpp

@@ -40,7 +40,7 @@ TEST(SBLockTest, LockTest) {
 
   // Take the API lock.
   {
-    lldb::SBLock lock = target.GetAPILock();
+    lldb::SBLock lock = target.AcquireAPILock();
     ASSERT_FALSE(locked.exchange(true));
 
     // Wake up the test thread.