[lldb] Delete the SharingPtr class

Summary:
The only use of this class was to implement the SharedCluster of ValueObjects.
However, the same functionality can be implemented using a regular
std::shared_ptr, and its little-known "sub-object pointer" feature, where the
pointer can point to one thing, but actually delete something else when it goes
out of scope.

This patch reimplements SharedCluster using this feature --
SharedClusterPointer::GetObject now returns a std::shared_pointer which points
to the ValueObject, but actually owns the whole cluster. The only change I
needed to make here is that now the SharedCluster object needs to be created
before the root ValueObject. This means that all private ValueObject
constructors get a ClusterManager argument, and their static Create functions do
the create-a-manager-and-pass-it-to-value-object dance.

Reviewers: teemperor, JDevlieghere, jingham

Subscribers: mgorny, jfb, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D74153

lldb/cmake/modules/LLDBFramework.cmake

@@ -59,8 +59,7 @@ list(REMOVE_ITEM root_public_headers ${root_private_headers})
 set(lldb_header_staging ${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders)
 foreach(header
     ${public_headers}
-    ${root_public_headers}
-    ${LLDB_SOURCE_DIR}/include/lldb/Utility/SharingPtr.h)
+    ${root_public_headers})
 
   get_filename_component(basename ${header} NAME)
   set(staged_header ${lldb_header_staging}/${basename})

lldb/include/lldb/Core/ValueObject.h

@@ -902,7 +902,7 @@ class ValueObject : public UserID {
   // Use this constructor to create a "root variable object".  The ValueObject
   // will be locked to this context through-out its lifespan.
 
-  ValueObject(ExecutionContextScope *exe_scope,
+  ValueObject(ExecutionContextScope *exe_scope, ValueObjectManager &manager,
               AddressType child_ptr_or_ref_addr_type = eAddressTypeLoad);
 
   // Use this constructor to create a ValueObject owned by another ValueObject.

lldb/include/lldb/Core/ValueObjectConstResult.h

@@ -121,30 +121,34 @@ class ValueObjectConstResult : public ValueObject {
   friend class ValueObjectConstResultImpl;
 
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
+                         ValueObjectManager &manager,
                          lldb::ByteOrder byte_order, uint32_t addr_byte_size,
                          lldb::addr_t address);
 
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
-                         const CompilerType &compiler_type,
-                         ConstString name, const DataExtractor &data,
-                         lldb::addr_t address);
+                         ValueObjectManager &manager,
+                         const CompilerType &compiler_type, ConstString name,
+                         const DataExtractor &data, lldb::addr_t address);
 
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
-                         const CompilerType &compiler_type,
-                         ConstString name,
+                         ValueObjectManager &manager,
+                         const CompilerType &compiler_type, ConstString name,
                          const lldb::DataBufferSP &result_data_sp,
                          lldb::ByteOrder byte_order, uint32_t addr_size,
                          lldb::addr_t address);
 
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
-                         const CompilerType &compiler_type,
-                         ConstString name, lldb::addr_t address,
-                         AddressType address_type, uint32_t addr_byte_size);
+                         ValueObjectManager &manager,
+                         const CompilerType &compiler_type, ConstString name,
+                         lldb::addr_t address, AddressType address_type,
+                         uint32_t addr_byte_size);
 
-  ValueObjectConstResult(ExecutionContextScope *exe_scope, const Value &value,
+  ValueObjectConstResult(ExecutionContextScope *exe_scope,
+                         ValueObjectManager &manager, const Value &value,
                          ConstString name, Module *module = nullptr);
 
-  ValueObjectConstResult(ExecutionContextScope *exe_scope, const Status &error);
+  ValueObjectConstResult(ExecutionContextScope *exe_scope,
+                         ValueObjectManager &manager, const Status &error);
 
   DISALLOW_COPY_AND_ASSIGN(ValueObjectConstResult);
 };

lldb/include/lldb/Core/ValueObjectDynamicValue.h

@@ -14,7 +14,6 @@
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Utility/ConstString.h"
-#include "lldb/Utility/SharingPtr.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"

lldb/include/lldb/Core/ValueObjectMemory.h

@@ -64,10 +64,12 @@ class ValueObjectMemory : public ValueObject {
   CompilerType m_compiler_type;
 
 private:
-  ValueObjectMemory(ExecutionContextScope *exe_scope, llvm::StringRef name,
+  ValueObjectMemory(ExecutionContextScope *exe_scope,
+                    ValueObjectManager &manager, llvm::StringRef name,
                     const Address &address, lldb::TypeSP &type_sp);
 
-  ValueObjectMemory(ExecutionContextScope *exe_scope, llvm::StringRef name,
+  ValueObjectMemory(ExecutionContextScope *exe_scope,
+                    ValueObjectManager &manager, llvm::StringRef name,
                     const Address &address, const CompilerType &ast_type);
   // For ValueObject only
   DISALLOW_COPY_AND_ASSIGN(ValueObjectMemory);

lldb/include/lldb/Core/ValueObjectRegister.h

@@ -69,6 +69,7 @@ class ValueObjectRegisterSet : public ValueObject {
   friend class ValueObjectRegisterContext;
 
   ValueObjectRegisterSet(ExecutionContextScope *exe_scope,
+                         ValueObjectManager &manager,
                          lldb::RegisterContextSP &reg_ctx_sp, uint32_t set_idx);
 
   // For ValueObject only
@@ -123,6 +124,7 @@ class ValueObjectRegister : public ValueObject {
   ValueObjectRegister(ValueObject &parent, lldb::RegisterContextSP &reg_ctx_sp,
                       uint32_t reg_num);
   ValueObjectRegister(ExecutionContextScope *exe_scope,
+                      ValueObjectManager &manager,
                       lldb::RegisterContextSP &reg_ctx_sp, uint32_t reg_num);
 
   // For ValueObject only

lldb/include/lldb/Core/ValueObjectVariable.h

@@ -77,6 +77,7 @@ class ValueObjectVariable : public ValueObject {
 
 private:
   ValueObjectVariable(ExecutionContextScope *exe_scope,
+                      ValueObjectManager &manager,
                       const lldb::VariableSP &var_sp);
   // For ValueObject only
   DISALLOW_COPY_AND_ASSIGN(ValueObjectVariable);

lldb/include/lldb/Utility/SharedCluster.h

@@ -10,46 +10,24 @@
 #define utility_SharedCluster_h_
 
 #include "lldb/Utility/LLDBAssert.h"
-#include "lldb/Utility/SharingPtr.h"
-
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 
+#include <memory>
 #include <mutex>
 
 namespace lldb_private {
 
-namespace imp {
-template <typename T>
-class shared_ptr_refcount : public lldb_private::imp::shared_count {
+template <class T>
+class ClusterManager : public std::enable_shared_from_this<ClusterManager<T>> {
 public:
-  template <class Y>
-  shared_ptr_refcount(Y *in) : shared_count(0), manager(in) {}
-
-  shared_ptr_refcount() : shared_count(0) {}
-
-  ~shared_ptr_refcount() override {}
-
-  void on_zero_shared() override { manager->DecrementRefCount(); }
-
-private:
-  T *manager;
-};
-
-} // namespace imp
-
-template <class T> class ClusterManager {
-public:
-  ClusterManager() : m_objects(), m_external_ref(0), m_mutex() {}
+  static std::shared_ptr<ClusterManager> Create() {
+    return std::shared_ptr<ClusterManager>(new ClusterManager());
+  }
 
   ~ClusterManager() {
     for (T *obj : m_objects)
       delete obj;
-
-    // Decrement refcount should have been called on this ClusterManager, and
-    // it should have locked the mutex, now we will unlock it before we destroy
-    // it...
-    m_mutex.unlock();
   }
 
   void ManageObject(T *new_object) {
@@ -59,33 +37,20 @@ template <class T> class ClusterManager {
     m_objects.push_back(new_object);
   }
 
-  typename lldb_private::SharingPtr<T> GetSharedPointer(T *desired_object) {
-    {
-      std::lock_guard<std::mutex> guard(m_mutex);
-      m_external_ref++;
-      if (!llvm::is_contained(m_objects, desired_object)) {
-        lldbassert(false && "object not found in shared cluster when expected");
-        desired_object = nullptr;
-      }
+  std::shared_ptr<T> GetSharedPointer(T *desired_object) {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    auto this_sp = this->shared_from_this();
+    if (!llvm::is_contained(m_objects, desired_object)) {
+      lldbassert(false && "object not found in shared cluster when expected");
+      desired_object = nullptr;
     }
-    return typename lldb_private::SharingPtr<T>(
-        desired_object, new imp::shared_ptr_refcount<ClusterManager>(this));
+    return {std::move(this_sp), desired_object};
   }
 
 private:
-  void DecrementRefCount() {
-    m_mutex.lock();
-    m_external_ref--;
-    if (m_external_ref == 0)
-      delete this;
-    else
-      m_mutex.unlock();
-  }
-
-  friend class imp::shared_ptr_refcount<ClusterManager>;
+  ClusterManager() : m_objects(), m_mutex() {}
 
   llvm::SmallVector<T *, 16> m_objects;
-  int m_external_ref;
   std::mutex m_mutex;
 };