[lldb] Never print children if the max depth has been reached

[augusto2112]

Summary

When formatting a variable, the max depth would potentially be ignored
if the current value object failed to print itself. Change that to
always respect the max depth, even if failure occurs

rdar://109855463

[augusto2112]

Remove failure check

[aprantl]

Would that be testable by implementing a python data formatter for a C struct that unconditionally returns an error?

lldb/include/lldb/DataFormatters/ValueObjectPrinter.h

protected:
   void GetValueSummaryError(std::string &value, std::string &summary,
                             std::string &error);
 
   bool PrintValueAndSummaryIfNeeded(bool &value_printed, bool &summary_printed);
 
   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();
 
   ValueObject *GetValueObjectForChildrenGeneration();
 
   void PrintChildrenPreamble(bool value_printed, bool summary_printed);
 
   void PrintChildrenPostamble(bool print_dotdotdot);

lldb/source/DataFormatters/ValueObjectPrinter.cpp

case Mode::Default:
     return m_count > 0;
   case Mode::Never:
     return false;
   }
   return false;
 }
 
 bool ValueObjectPrinter::ShouldPrintChildren(
-    bool is_failed_description,
     DumpValueObjectOptions::PointerDepth &curr_ptr_depth) {
   const bool is_ref = IsRef();
   const bool is_ptr = IsPtr();
   const bool is_uninit = IsUninitialized();
 
   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)
     return true;
 
   if (m_options.m_use_objc)
     return false;
 
   bool print_children = true;
   if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
     print_children = type_summary->DoesPrintChildren(m_valobj);
 
-  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).
 
-    // Use a new temporary pointer depth in case we override the current
-    // pointer depth below...
 
-    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;
 
-      const bool is_root_level = m_curr_depth == 0;
 
-      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 curr_ptr_depth.CanAllowExpansion();
-    }
 
-    return print_children || m_summary.empty();
-  }
-  return false;
-}
 
 bool ValueObjectPrinter::ShouldExpandEmptyAggregates() {
   TypeSummaryImpl *entry = GetSummaryFormatter();
 
   if (!entry)
     return true;
 
   return entry->DoesPrintEmptyAggregates();
if (num_children) {
     else
       m_stream->PutChar(')');
   }
   return true;
 }
 
 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 ||
        (m_options.m_pointer_as_array) || m_options.m_show_location)
           ? false
           : DataVisualization::ShouldPrintAsOneLiner(*m_valobj);
   if (print_children && IsInstancePointer()) {
     uint64_t instance_ptr_value = m_valobj->GetValueAsUnsigned(0);

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

 """
 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")
+            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

 
 struct C {
   B b;
 };
 
 int main() {
   C *c = new C[5];
   puts("break here");
-  return 0; // break here
+  return 0;
 }