Merge pull request #6986 from augusto2112/cherry-pick-data-format-recursion

Cherry pick data format recursion fix

Author: adrian-prantl

lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h

@@ -25,7 +25,7 @@ class DumpValueObjectOptions {
     enum class Mode { Always, Formatters, Default, Never } m_mode;
     uint32_t m_count;
 
-    PointerDepth operator--() const {
+    PointerDepth Decremented() const {
       if (m_count > 0)
         return PointerDepth{m_mode, m_count - 1};
       return PointerDepth{m_mode, m_count};

lldb/include/lldb/DataFormatters/ValueObjectPrinter.h

@@ -91,8 +91,7 @@ class ValueObjectPrinter {
   bool PrintObjectDescriptionIfNeeded(bool value_printed, bool summary_printed);
 
   bool
-  ShouldPrintChildren(bool is_failed_description,
-                      DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
+  ShouldPrintChildren(DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
   bool ShouldExpandEmptyAggregates();
 

lldb/source/DataFormatters/ValueObjectPrinter.cpp

@@ -523,7 +523,6 @@ bool DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const {
 }
 
 bool ValueObjectPrinter::ShouldPrintChildren(
-    bool is_failed_description,
     DumpValueObjectOptions::PointerDepth &curr_ptr_depth) {
   const bool is_ref = IsRef();
   const bool is_ptr = IsPtr();
@@ -532,6 +531,10 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (is_uninit)
     return false;
 
+  // If we have reached the maximum depth we shouldn't print any more children.
+  if (HasReachedMaximumDepth())
+    return false;
+
   // if the user has specified an element count, always print children as it is
   // explicit user demand being honored
   if (m_options.m_pointer_as_array)
@@ -542,38 +545,37 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (m_options.m_use_objc)
     return false;
 
-  if (is_failed_description || !HasReachedMaximumDepth()) {
-    // We will show children for all concrete types. We won't show pointer
-    // contents unless a pointer depth has been specified. We won't reference
-    // contents unless the reference is the root object (depth of zero).
+  bool print_children = true;
+  if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
+    print_children = type_summary->DoesPrintChildren(m_valobj);
 
-    // Use a new temporary pointer depth in case we override the current
-    // pointer depth below...
+  // We will show children for all concrete types. We won't show pointer
+  // contents unless a pointer depth has been specified. We won't reference
+  // contents unless the reference is the root object (depth of zero).
 
-    if (is_ptr || is_ref) {
-      // We have a pointer or reference whose value is an address. Make sure
-      // that address is not NULL
-      AddressType ptr_address_type;
-      if (m_valobj->GetPointerValue(&ptr_address_type) == 0)
-        return false;
+  // Use a new temporary pointer depth in case we override the current
+  // pointer depth below...
 
-      const bool is_root_level = m_curr_depth == 0;
+  if (is_ptr || is_ref) {
+    // We have a pointer or reference whose value is an address. Make sure
+    // that address is not NULL
+    AddressType ptr_address_type;
+    if (m_valobj->GetPointerValue(&ptr_address_type) == 0)
+      return false;
 
-      if (is_ref && is_root_level) {
-        // If this is the root object (depth is zero) that we are showing and
-        // it is a reference, and no pointer depth has been supplied print out
-        // what it references. Don't do this at deeper depths otherwise we can
-        // end up with infinite recursion...
-        return true;
-      }
+    const bool is_root_level = m_curr_depth == 0;
 
-      return curr_ptr_depth.CanAllowExpansion(false, entry, m_valobj,
-                                              m_summary);
+    if (is_ref && is_root_level && print_children) {
+      // If this is the root object (depth is zero) that we are showing and
+      // it is a reference, and no pointer depth has been supplied print out
+      // what it references. Don't do this at deeper depths otherwise we can
+      // end up with infinite recursion...
+      return true;
     }
-
-    return (!entry || entry->DoesPrintChildren(m_valobj) || m_summary.empty());
+    return curr_ptr_depth.CanAllowExpansion(false, entry, m_valobj, m_summary);
   }
-  return false;
+
+  return print_children || m_summary.empty();
 }
 
 bool ValueObjectPrinter::ShouldExpandEmptyAggregates() {
@@ -610,7 +612,7 @@ void ValueObjectPrinter::PrintChildrenPreamble(bool value_printed,
 void ValueObjectPrinter::PrintChild(
     ValueObjectSP child_sp,
     const DumpValueObjectOptions::PointerDepth &curr_ptr_depth) {
-  const uint32_t consumed_depth = (!m_options.m_pointer_as_array) ? 1 : 0;
+  const uint32_t consumed_summary_depth = m_options.m_pointer_as_array ? 0 : 1;
   const bool does_consume_ptr_depth =
       ((IsPtr() && !m_options.m_pointer_as_array) || IsRef());
 
@@ -623,15 +625,18 @@ void ValueObjectPrinter::PrintChild(
       .SetHideValue(m_options.m_hide_value)
       .SetOmitSummaryDepth(child_options.m_omit_summary_depth > 1
                                ? child_options.m_omit_summary_depth -
-                                     consumed_depth
+                                     consumed_summary_depth
                                : 0)
       .SetElementCount(0);
 
   if (child_sp.get()) {
-    ValueObjectPrinter child_printer(
-        child_sp.get(), m_stream, child_options,
-        does_consume_ptr_depth ? --curr_ptr_depth : curr_ptr_depth,
-        m_curr_depth + consumed_depth, m_printed_instance_pointers);
+    auto ptr_depth = curr_ptr_depth;
+    if (does_consume_ptr_depth)
+      ptr_depth = curr_ptr_depth.Decremented();
+
+    ValueObjectPrinter child_printer(child_sp.get(), m_stream, child_options,
+                                     ptr_depth, m_curr_depth + 1,
+                                     m_printed_instance_pointers);
     child_printer.PrintValueObject();
   }
 }
@@ -803,14 +808,10 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) {
 
 void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
                                                bool summary_printed) {
-  // This flag controls whether we tried to display a description for this
-  // object and failed if that happens, we want to display the children if any.
-  bool is_failed_description =
-      !PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
+  PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
 
   DumpValueObjectOptions::PointerDepth curr_ptr_depth = m_ptr_depth;
-  const bool print_children =
-      ShouldPrintChildren(is_failed_description, curr_ptr_depth);
+  const bool print_children = ShouldPrintChildren(curr_ptr_depth);
   const bool print_oneline =
       (curr_ptr_depth.CanAllowExpansion() || m_options.m_show_types ||
        !m_options.m_allow_oneliner_mode || m_options.m_flat_output ||

lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/Makefile

@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/TestFrameVarDepthAndElemCount.py

@@ -0,0 +1,27 @@
+"""
+Tests that frame variable --depth and --element-count options work correctly
+together
+"""
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestFrameVarDepthAndElemCount(TestBase):
+    def test(self):
+        """Test that bool types work in the expression parser"""
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.cpp")
+        )
+
+        # Check that we print 5 elements but only 2 levels deep.
+        self.expect('frame var --depth 2 --element-count 5 -- c', 
+        substrs=[
+            '[0] = {\n    b ={...}\n  }',
+            '[1] = {\n    b ={...}\n  }',
+            '[2] = {\n    b ={...}\n  }',
+            '[3] = {\n    b ={...}\n  }',
+            '[4] = {\n    b ={...}\n  }',
+            ])
+

lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp

@@ -0,0 +1,19 @@
+#include <cstdio>
+
+struct A {
+  int i = 42;
+};
+
+struct B {
+  A a;
+};
+
+struct C {
+  B b;
+};
+
+int main() {
+  C *c = new C[5];
+  puts("break here");
+  return 0;
+}