fixup! remove early-return for empty shared_ptrs; refactor weak/strong count printing

Author: Michael137

lldb/include/lldb/DataFormatters/FormattersHelpers.h

@@ -55,10 +55,9 @@ void AddFilter(TypeCategoryImpl::SharedPointer category_sp,
 
 std::optional<size_t> ExtractIndexFromString(const char *item_name);
 
-/// Returns \c false if the smart pointer formatters shouldn't continue
-/// formatting the rest of the object. Currently this is the case when the
-/// pointer is a nullptr.
-bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
+/// Prints the summary for the pointer value of a C++
+/// std::unique_ptr/std::shared_ptr/std::weak_ptr.
+void DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
                                    const TypeSummaryOptions &options);
 
 Address GetArrayAddressOrPointerValue(ValueObject &valobj);

lldb/source/DataFormatters/FormattersHelpers.cpp

@@ -127,24 +127,21 @@ lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) {
   return data_addr.address;
 }
 
-bool lldb_private::formatters::DumpCxxSmartPtrPointerSummary(
+void lldb_private::formatters::DumpCxxSmartPtrPointerSummary(
     Stream &stream, ValueObject &ptr, const TypeSummaryOptions &options) {
   if (ptr.GetValueAsUnsigned(0) == 0) {
     stream.Printf("nullptr");
-    return true;
+    return;
   }
 
   Status error;
   ValueObjectSP pointee_sp = ptr.Dereference(error);
-  // FIXME: should we be handling this as an error?
   if (!pointee_sp || !error.Success())
-    return false;
+    return;
 
   if (!pointee_sp->DumpPrintableRepresentation(
           stream, ValueObject::eValueObjectRepresentationStyleSummary,
           lldb::eFormatInvalid,
           ValueObject::PrintableRepresentationSpecialCases::eDisable, false))
     stream.Printf("ptr = 0x%" PRIx64, ptr.GetValueAsUnsigned(0));
-
-  return false;
 }

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

@@ -157,23 +157,38 @@ bool lldb_private::formatters::LibcxxSmartPointerSummaryProvider(
   ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
   if (!valobj_sp)
     return false;
-  ValueObjectSP ptr_sp(valobj_sp->GetChildMemberWithName("__ptr_"));
-  ValueObjectSP count_sp(
-      valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_owners_"}));
-  ValueObjectSP weakcount_sp(
-      valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_weak_owners_"}));
 
-  if (!ptr_sp)
+  ValueObjectSP ptr_sp(valobj_sp->GetChildMemberWithName("__ptr_"));
+  ValueObjectSP ctrl_sp(valobj_sp->GetChildMemberWithName("__cntrl_"));
+  if (!ctrl_sp || !ptr_sp)
     return false;
 
-  if (!DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options))
+  DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options);
+
+  bool success;
+  uint64_t ctrl_addr = ctrl_sp->GetValueAsUnsigned(0, &success);
+  // Empty control field. We're done.
+  if (!success || ctrl_addr == 0)
     return true;
 
-  if (count_sp)
-    stream.Printf(" strong=%" PRIu64, 1 + count_sp->GetValueAsUnsigned(0));
+  if (auto count_sp = ctrl_sp->GetChildMemberWithName("__shared_owners_")) {
+    bool success;
+    uint64_t count = count_sp->GetValueAsUnsigned(0, &success);
+    if (!success)
+      return false;
+
+    stream.Printf(" strong=%" PRIu64, count + 1);
+  }
 
-  if (weakcount_sp)
-    stream.Printf(" weak=%" PRIu64, 1 + weakcount_sp->GetValueAsUnsigned(0));
+  if (auto weak_count_sp =
+          ctrl_sp->GetChildMemberWithName("__shared_weak_owners_")) {
+    bool success;
+    uint64_t count = weak_count_sp->GetValueAsUnsigned(0, &success);
+    if (!success)
+      return false;
+
+    stream.Printf(" weak=%" PRIu64, count + 1);
+  }
 
   return true;
 }

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py

@@ -91,6 +91,20 @@ def test_shared_ptr_variables(self):
         self.expect_var_path("sp_user->id", type="int", value="30")
         self.expect_var_path("sp_user->name", type="std::string", summary='"steph"')
 
+        #valobj = self.expect_var_path(
+        #    "si",
+        #    type="std::shared_ptr<int> &",
+        #    children=[ValueCheck(name="__ptr_")],
+        #)
+        #self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+
+        #valobj = self.expect_var_path(
+        #    "sie",
+        #    type="std::shared_ptr<int> &",
+        #    children=[ValueCheck(name="__ptr_")],
+        #)
+        #self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$")
+
         self.runCmd("settings set target.experimental.use-DIL true")
         self.expect_var_path("ptr_node->value", value="1")
         self.expect_var_path("ptr_node->next->value", value="2")

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp

@@ -22,5 +22,9 @@ int main() {
       std::shared_ptr<NodeS>(new NodeS{nullptr, 2});
   ptr_node = std::shared_ptr<NodeS>(new NodeS{std::move(ptr_node), 1});
 
+  // Construct empty shared_ptr with non-null control field.
+  std::shared_ptr<int> si(new int(47));
+  std::shared_ptr<int> sie(si, nullptr);
+
   return 0; // break here
 }