[lldb] Show more children of top level values

[kastiglione]

This changes printing values at the top level (such as variables). The original default
value for target.max-children-count was 256. The default has since been reduced, to
avoid printing too much data (ff12762).

However, users will naturally have expectations that all (or significantly many)
children are shown for top level values. The following code keeps 256 as the max count
for top level values (unless customized). The value of target.max-children-count will
continue to apply to nested values (depth >= 1).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)

Changes

This changes printing values at the top level (such as variables). The original default
value for target.max-children-count was 256. The default has since been reduced, to
avoid printing too much data (ff12762).

However, users will naturally have expectations that all (or significantly many)
children are shown for top level values. The following code keeps 256 as the max count
for top level values (unless customized). The value of target.max-children-count will
continue to apply to nested values (depth >= 1).

---

Full diff: https://github.com/llvm/llvm-project/pull/140938.diff

7 Files Affected:

• (modified) lldb/include/lldb/Target/Target.h (+1-1)
• (modified) lldb/source/API/SBTarget.cpp (+1-1)
• (modified) lldb/source/Core/FormatEntity.cpp (+1-1)
• (modified) lldb/source/DataFormatters/FormatManager.cpp (+1-1)
• (modified) lldb/source/DataFormatters/ValueObjectPrinter.cpp (+23-4)
• (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp (+1-1)
• (modified) lldb/source/Target/Target.cpp (+6-3)

diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 0d4e11b65339e..bcde310378acf 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -186,7 +186,7 @@ class TargetProperties : public Properties {
 
   uint32_t GetMaxZeroPaddingInFloatFormat() const;
 
-  uint32_t GetMaximumNumberOfChildrenToDisplay() const;
+  std::pair<uint32_t, bool> GetMaximumNumberOfChildrenToDisplay() const;
 
   /// Get the max depth value, augmented with a bool to indicate whether the
   /// depth is the default.
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index cd8a770a0ec04..c3e3b6105d56f 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1713,7 +1713,7 @@ uint32_t SBTarget::GetMaximumNumberOfChildrenToDisplay() const {
 
   TargetSP target_sp(GetSP());
   if(target_sp){
-     return target_sp->GetMaximumNumberOfChildrenToDisplay();
+    return target_sp->GetMaximumNumberOfChildrenToDisplay().first;
   }
   return 0;
 }
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index 4f2d39873c7fb..98e5bfe37cd62 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1025,7 +1025,7 @@ static bool DumpValue(Stream &s, const SymbolContext *sc,
     if (index_higher < 0)
       index_higher = valobj->GetNumChildrenIgnoringErrors() - 1;
 
-    uint32_t max_num_children =
+    auto [max_num_children, _] =
         target->GetTargetSP()->GetMaximumNumberOfChildrenToDisplay();
 
     bool success = true;
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index 3b891cecb1c8b..feb69840bc64b 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -458,7 +458,7 @@ bool FormatManager::ShouldPrintAsOneLiner(ValueObject &valobj) {
   if (valobj.GetSummaryFormat().get() != nullptr)
     return valobj.GetSummaryFormat()->IsOneLiner();
 
-  const size_t max_num_children =
+  const auto [max_num_children, _] =
       (target_sp ? *target_sp : Target::GetGlobalProperties())
           .GetMaximumNumberOfChildrenToDisplay();
   auto num_children = valobj.GetNumChildren(max_num_children);
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 5e04a621bbda8..4890aa8f7ee66 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -633,6 +633,27 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
+static uint32_t determineMaxChildren(bool ignore_cap, uint32_t cur_depth,
+                                     TargetSP target_sp) {
+  if (ignore_cap)
+    return UINT32_MAX;
+
+  const auto [max_num_children, max_is_default] =
+      target_sp->GetMaximumNumberOfChildrenToDisplay();
+
+  // Special handling for printing values at the top level (such as variables).
+  // The original default value for target.max-children-count was 256. The
+  // default has since been reduced, to avoid printing too much data. However,
+  // users will naturally have expectations that all children are shown for top
+  // level values. The following code keeps 256 as the max count for top level
+  // values (unless customized).
+  const uint32_t top_level_max_num_childen = 256;
+  if (cur_depth == 0 && max_is_default)
+    return top_level_max_num_childen;
+
+  return max_num_children;
+}
+
 llvm::Expected<uint32_t>
 ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
@@ -641,10 +662,8 @@ ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
     return m_options.m_pointer_as_array.m_element_count;
 
   const uint32_t max_num_children =
-      m_options.m_ignore_cap ? UINT32_MAX
-                             : GetMostSpecializedValue()
-                                   .GetTargetSP()
-                                   ->GetMaximumNumberOfChildrenToDisplay();
+      determineMaxChildren(m_options.m_ignore_cap, m_curr_depth,
+                           GetMostSpecializedValue().GetTargetSP());
   // Ask for one more child than the maximum to see if we should print "...".
   auto num_children_or_err = synth_valobj.GetNumChildren(
       llvm::SaturatingAdd(max_num_children, uint32_t(1)));
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
index 30db5f15c388f..87b66697feae9 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -176,7 +176,7 @@ lldb::ChildCacheState AbstractListFrontEnd::Update() {
 
   if (m_backend.GetTargetSP())
     m_list_capping_size =
-        m_backend.GetTargetSP()->GetMaximumNumberOfChildrenToDisplay();
+        m_backend.GetTargetSP()->GetMaximumNumberOfChildrenToDisplay().first;
   if (m_list_capping_size == 0)
     m_list_capping_size = 255;
 
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 9660fc97970b0..6e2ca57072fa4 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4814,10 +4814,13 @@ uint32_t TargetProperties::GetMaxZeroPaddingInFloatFormat() const {
       idx, g_target_properties[idx].default_uint_value);
 }
 
-uint32_t TargetProperties::GetMaximumNumberOfChildrenToDisplay() const {
+std::pair<uint32_t, bool>
+TargetProperties::GetMaximumNumberOfChildrenToDisplay() const {
   const uint32_t idx = ePropertyMaxChildrenCount;
-  return GetPropertyAtIndexAs<uint64_t>(
-      idx, g_target_properties[idx].default_uint_value);
+  auto *option_value =
+      m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(idx);
+  bool is_default = !option_value->OptionWasSet();
+  return {option_value->GetCurrentValue(), is_default};
 }
 
 std::pair<uint32_t, bool>

[jimingham]

This behavior has gotten fairly complicated, you need to describe how it works somewhere that users can consult.

[kastiglione]

I've updated the help description for max-children-count to reflect how it is applied.

[kastiglione]

@jimingham is the updated description for max-children-count sufficient?

[jimingham]

One other thing, if I set a custom value and want to get the default behavior back, how do I do that? I think that defaults clear max-children-count should do it, but it would be worth checking. If it does, it might be worth giving that hint in the help, since that's not obvious.