cameron314 updated this revision to Diff 223919.
cameron314 added a comment.

Added comments.


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

https://reviews.llvm.org/D68641

Files:
  source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
  source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -39,9 +39,14 @@
   bool GetSummary(Stream &stream, const TypeSummaryOptions &options);
 
 private:
-  ValueObjectSP m_ptr_obj;
-  ValueObjectSP m_obj_obj;
-  ValueObjectSP m_del_obj;
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr;
+  ValueObject* m_obj_obj = nullptr;
+  ValueObject* m_del_obj = nullptr;
 
   ValueObjectSP GetTuple();
 };
@@ -92,17 +97,17 @@
 
   ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
   if (ptr_obj)
-    m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
+    m_ptr_obj = ptr_obj->Clone(ConstString("pointer")).get();
 
   ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
   if (del_obj)
-    m_del_obj = del_obj->Clone(ConstString("deleter"));
+    m_del_obj = del_obj->Clone(ConstString("deleter")).get();
 
   if (m_ptr_obj) {
     Status error;
     ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
     if (error.Success()) {
-      m_obj_obj = obj_obj->Clone(ConstString("object"));
+      m_obj_obj = obj_obj->Clone(ConstString("object")).get();
     }
   }
 
@@ -114,11 +119,11 @@
 lldb::ValueObjectSP
 LibStdcppUniquePtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx == 0)
-    return m_ptr_obj;
+    return m_ptr_obj->GetSP();
   if (idx == 1)
-    return m_del_obj;
+    return m_del_obj->GetSP();
   if (idx == 2)
-    return m_obj_obj;
+    return m_obj_obj->GetSP();
   return lldb::ValueObjectSP();
 }
 
Index: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -37,7 +37,12 @@
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  std::vector<ValueObjectSP> m_members;
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  std::vector<ValueObject*> m_members;
 };
 
 } // end of anonymous namespace
@@ -72,7 +77,7 @@
         if (value_sp) {
           StreamString name;
           name.Printf("[%zd]", m_members.size());
-          m_members.push_back(value_sp->Clone(ConstString(name.GetString())));
+          m_members.push_back(value_sp->Clone(ConstString(name.GetString())).get());
         }
       }
     }
@@ -86,7 +91,7 @@
 lldb::ValueObjectSP
 LibStdcppTupleSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx < m_members.size())
-    return m_members[idx];
+    return m_members[idx]->GetSP();
   return lldb::ValueObjectSP();
 }
 
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -184,7 +184,6 @@
 
 private:
   size_t m_size = 0;
-  ValueObjectSP m_base_sp;
 };
 } // namespace
 
Index: source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
@@ -30,47 +30,56 @@
   ValueObjectSP GetChildAtIndex(size_t idx) override;
 
 private:
-  std::vector<ValueObjectSP> m_elements;
-  ValueObjectSP m_base_sp;
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  std::vector<ValueObject*> m_elements;
+  ValueObject* m_base = nullptr;
 };
 }
 
 bool TupleFrontEnd::Update() {
   m_elements.clear();
-  m_base_sp = m_backend.GetChildMemberWithName(ConstString("__base_"), true);
-  if (! m_base_sp) {
+  m_base = nullptr;
+
+  ValueObjectSP base_sp;
+  base_sp = m_backend.GetChildMemberWithName(ConstString("__base_"), true);
+  if (!base_sp) {
     // Pre r304382 name of the base element.
-    m_base_sp = m_backend.GetChildMemberWithName(ConstString("base_"), true);
+    base_sp = m_backend.GetChildMemberWithName(ConstString("base_"), true);
   }
-  if (! m_base_sp)
+  if (!base_sp)
     return false;
-  m_elements.assign(m_base_sp->GetCompilerType().GetNumDirectBaseClasses(),
-                    ValueObjectSP());
+  m_base = base_sp.get();
+  m_elements.assign(base_sp->GetCompilerType().GetNumDirectBaseClasses(),
+                    nullptr);
   return false;
 }
 
 ValueObjectSP TupleFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx >= m_elements.size())
     return ValueObjectSP();
-  if (!m_base_sp)
+  if (!m_base)
     return ValueObjectSP();
   if (m_elements[idx])
-    return m_elements[idx];
+    return m_elements[idx]->GetSP();
 
   CompilerType holder_type =
-      m_base_sp->GetCompilerType().GetDirectBaseClassAtIndex(idx, nullptr);
+      m_base->GetCompilerType().GetDirectBaseClassAtIndex(idx, nullptr);
   if (!holder_type)
     return ValueObjectSP();
-  ValueObjectSP holder_sp = m_base_sp->GetChildAtIndex(idx, true);
+  ValueObjectSP holder_sp = m_base->GetChildAtIndex(idx, true);
   if (!holder_sp)
     return ValueObjectSP();
 
   ValueObjectSP elem_sp = holder_sp->GetChildAtIndex(0, true);
   if (elem_sp)
     m_elements[idx] =
-        elem_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str()));
+        elem_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str())).get();
 
-  return m_elements[idx];
+  return m_elements[idx]->GetSP();
 }
 
 SyntheticChildrenFrontEnd *
Index: source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
@@ -38,16 +38,21 @@
   }
 
 private:
-  ValueObjectSP m_container_sp;
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_container_sp = nullptr;
 };
 } // namespace
 
 bool QueueFrontEnd::Update() {
-  m_container_sp.reset();
+  m_container_sp = nullptr;
   ValueObjectSP c_sp = m_backend.GetChildMemberWithName(ConstString("c"), true);
   if (!c_sp)
     return false;
-  m_container_sp = c_sp->GetSyntheticValue();
+  m_container_sp = c_sp->GetSyntheticValue().get();
   return false;
 }
 
Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -31,7 +31,6 @@
 
 private:
   size_t m_size = 0;
-  ValueObjectSP m_base_sp;
 };
 } // namespace
 
Index: source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
@@ -30,8 +30,15 @@
   ValueObjectSP GetChildAtIndex(size_t idx) override;
 
 private:
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  // Value objects created from raw data (i.e. in a different cluster) must
+  // be referenced via shared pointer to keep them alive, however.
   std::vector<ValueObjectSP> m_elements;
-  ValueObjectSP m_first;
+  ValueObject* m_first = nullptr;
   CompilerType m_bool_type;
   ByteOrder m_byte_order = eByteOrderInvalid;
   uint8_t m_byte_size = 0;
@@ -50,7 +57,7 @@
 
 bool BitsetFrontEnd::Update() {
   m_elements.clear();
-  m_first.reset();
+  m_first = nullptr;
 
   TargetSP target_sp = m_backend.GetTargetSP();
   if (!target_sp)
@@ -63,7 +70,7 @@
 
   m_elements.assign(size, ValueObjectSP());
 
-  m_first = m_backend.GetChildMemberWithName(ConstString("__first_"), true);
+  m_first = m_backend.GetChildMemberWithName(ConstString("__first_"), true).get();
   return false;
 }
 
@@ -86,7 +93,7 @@
     chunk = m_first->GetChildAtIndex(idx / *bit_size, true);
   } else {
     type = m_first->GetCompilerType();
-    chunk = m_first;
+    chunk = m_first->GetSP();
   }
   if (!type || !chunk)
     return {};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to