[lldb] Add ability to hide the root name of a value

[kastiglione]

Summary

When printing a value, allow the root value's name to be elided, without omiting the
names of child values.

At the API level, this adds SetHideRootName(), which joins the existing
SetHideName() function.

This functionality is used by dwim-print and expression.

Fixes an issue identified by @jgorbe in https://reviews.llvm.org/D145609.

[jgorbe]

I've tried building lldb with this patch applied and it fixes the problem I saw. Thanks for the quick fix!

I'm not very familiar with the code so I'd appreciate if someone else gives their LGTM too but the changes look reasonable to me (just one small nit I've mentioned in an inline comment).

lldb/source/DataFormatters/ValueObjectPrinter.cpp

278

This method name reads like a command, rather than a predicate. What about something like ShouldShowName or ShouldPrintName? This would also match the style of other method names already in this file such as ShouldPrintValueObject.

[kastiglione]

Rename ShowName to ShouldShowName

[kastiglione]

lldb/source/DataFormatters/ValueObjectPrinter.cpp

278

Good call, ShouldShowName is better.

[jgorbe]

I just noticed a minor aesthetic problem with the input of dwim-print when using data formatters. There are some spacing adjustments in this commit but I'm not sure if they are the actual cause (please let me know if you'd prefer me to file a bug instead).

You can reproduce it by defining some random struct X and using settings set auto-one-line-summaries false like you did in the test case for this commit.

If you add a summary string:

type summary add -s "my summary" -e "X"

then the output of dwim-print without persistent results has the wrong spacing:

(lldb) dwim-print my_x
(X)  my summary{
  value = 0
}

(note there are two spaces before "my summary" and no space before the opening brace).

Using expr or dwim-print --persistent-result on -- doesn't have this problem:

(lldb) dwim-print --persistent-result on -- my_x
(X) $2 = my summary {
  value = 0
}

[kastiglione]

@jgorbe thanks for reporting. I have a fix for the missing space, I will look into the cause of the double space and fix that as well. I'll do this today.

lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h

public:
 
   DumpValueObjectOptions &
   SetSummary(lldb::TypeSummaryImplSP summary = lldb::TypeSummaryImplSP());
 
   DumpValueObjectOptions &SetRootValueObjectName(const char *name = nullptr);
 
   DumpValueObjectOptions &SetHideRootType(bool hide_root_type = false);
 
+  DumpValueObjectOptions &SetHideRootName(bool hide_root_name);
+
   DumpValueObjectOptions &SetHideName(bool hide_name = false);
 
   DumpValueObjectOptions &SetHideValue(bool hide_value = false);
 
   DumpValueObjectOptions &SetHidePointerValue(bool hide = false);
 
   DumpValueObjectOptions &SetVariableFormatDisplayLanguage(
       lldb::LanguageType lang = lldb::eLanguageTypeUnknown);
public:
   bool m_use_synthetic : 1;
   bool m_scope_already_checked : 1;
   bool m_flat_output : 1;
   bool m_ignore_cap : 1;
   bool m_show_types : 1;
   bool m_show_location : 1;
   bool m_use_objc : 1;
   bool m_hide_root_type : 1;
+  bool m_hide_root_name : 1;
   bool m_hide_name : 1;
   bool m_hide_value : 1;
   bool m_run_validator : 1;
   bool m_use_type_display_name : 1;
   bool m_allow_oneliner_mode : 1;
   bool m_hide_pointer_value : 1;
   bool m_reveal_empty_aggregates : 1;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_DATAFORMATTERS_DUMPVALUEOBJECTOPTIONS_H

lldb/include/lldb/DataFormatters/ValueObjectPrinter.h

protected:
 
   void PrintChildrenIfNeeded(bool value_printed, bool summary_printed);
 
   bool PrintChildrenOneLiner(bool hide_names);
 
   bool HasReachedMaximumDepth();
 
 private:
+  bool ShouldShowName() const;
+
   ValueObject *m_orig_valobj;
   ValueObject *m_valobj;
   Stream *m_stream;
   DumpValueObjectOptions m_options;
   Flags m_type_flags;
   CompilerType m_compiler_type;
   DumpValueObjectOptions::PointerDepth m_ptr_depth;
   uint32_t m_curr_depth;

lldb/source/Commands/CommandObjectDWIMPrint.cpp

bool CommandObjectDWIMPrint::DoExecute(StringRef command,
   // Fallback to the dummy target, which can allow for expression evaluation.
   Target &target = target_ptr ? *target_ptr : GetDummyTarget();
 
   const EvaluateExpressionOptions eval_options =
       m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
 
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
       m_expr_options.m_verbosity, m_format_options.GetFormat());
-  dump_options.SetHideName(eval_options.GetSuppressPersistentResult());
+  dump_options.SetHideRootName(eval_options.GetSuppressPersistentResult());
 
   // First, try `expr` as the name of a frame variable.
   if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
     auto valobj_sp = frame->FindVariable(ConstString(expr));
     if (valobj_sp && valobj_sp->GetError().Success()) {
       if (!eval_options.GetSuppressPersistentResult()) {
         if (auto persisted_valobj = valobj_sp->Persist())
           valobj_sp = persisted_valobj;

lldb/source/Commands/CommandObjectExpression.cpp

if (result_valobj_sp->GetError().Success()) {
                 "expression cannot be used with --element-count %s\n",
                 error.AsCString(""));
             return false;
           }
         }
 
         DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
             m_command_options.m_verbosity, format));
-        options.SetHideName(eval_options.GetSuppressPersistentResult());
+        options.SetHideRootName(eval_options.GetSuppressPersistentResult());
         options.SetVariableFormatDisplayLanguage(
             result_valobj_sp->GetPreferredDisplayLanguage());
 
         result_valobj_sp->Dump(output_stream, options);
 
         result.SetStatus(eReturnStatusSuccessFinishResult);
       }
     } else {

lldb/source/DataFormatters/DumpValueObjectOptions.cpp

 using namespace lldb_private;
 
 DumpValueObjectOptions::DumpValueObjectOptions()
     : m_summary_sp(), m_root_valobj_name(),
       m_max_ptr_depth(PointerDepth{PointerDepth::Mode::Default, 0}),
       m_decl_printing_helper(), m_pointer_as_array(), m_use_synthetic(true),
       m_scope_already_checked(false), m_flat_output(false), m_ignore_cap(false),
       m_show_types(false), m_show_location(false), m_use_objc(false),
-      m_hide_root_type(false), m_hide_name(false), m_hide_value(false),
-      m_run_validator(false), m_use_type_display_name(true),
-      m_allow_oneliner_mode(true), m_hide_pointer_value(false),
-      m_reveal_empty_aggregates(true) {}
+      m_hide_root_type(false), m_hide_root_name(false), m_hide_name(false),
+      m_hide_value(false), m_run_validator(false),
+      m_use_type_display_name(true), m_allow_oneliner_mode(true),
+      m_hide_pointer_value(false), m_reveal_empty_aggregates(true) {}
 
 DumpValueObjectOptions::DumpValueObjectOptions(ValueObject &valobj)
     : DumpValueObjectOptions() {
   m_use_dynamic = valobj.GetDynamicValueType();
   m_use_synthetic = valobj.IsSynthetic();
   m_varformat_language = valobj.GetPreferredDisplayLanguage();
 }
 

 }
 
 DumpValueObjectOptions &
 DumpValueObjectOptions::SetHideRootType(bool hide_root_type) {
   m_hide_root_type = hide_root_type;
   return *this;
 }
 
+DumpValueObjectOptions &
+DumpValueObjectOptions::SetHideRootName(bool hide_root_name) {
+  m_hide_root_name = hide_root_name;
+  return *this;
+}
+
 DumpValueObjectOptions &DumpValueObjectOptions::SetHideName(bool hide_name) {
   m_hide_name = hide_name;
   return *this;
 }
 
 DumpValueObjectOptions &DumpValueObjectOptions::SetHideValue(bool hide_value) {
   m_hide_value = hide_value;
   return *this;

lldb/source/DataFormatters/ValueObjectPrinter.cpp

if (type_name) {
         }
       }
       typeName << type_name_str.c_str();
     }
   }
 
   StreamString varName;
 
-  if (!m_options.m_hide_name) {
+  if (ShouldShowName()) {
     if (m_options.m_flat_output)
       m_valobj->GetExpressionPath(varName);
     else
       varName << GetRootNameForDisplay();
   }
 
   bool decl_printed = false;
   if (!m_options.m_decl_printing_helper) {
void ValueObjectPrinter::PrintDecl() {
   }
 
   // if the helper failed, or there is none, do a default thing
   if (!decl_printed) {
     if (!typeName.Empty())
       m_stream->Printf("(%s) ", typeName.GetData());
     if (!varName.Empty())
       m_stream->Printf("%s =", varName.GetData());
-    else if (!m_options.m_hide_name)
+    else if (ShouldShowName())
       m_stream->Printf(" =");
   }
 }
 
 bool ValueObjectPrinter::CheckScopeIfNeeded() {
   if (m_options.m_scope_already_checked)
     return true;
   return m_valobj->IsInScope();
if (m_error.size()) {
           (entry == nullptr ||
            (entry->DoesPrintValue(m_valobj) ||
             m_options.m_format != eFormatDefault) ||
            m_summary.empty()) &&
           !m_options.m_hide_value) {
         if (m_options.m_hide_pointer_value &&
             IsPointerValue(m_valobj->GetCompilerType())) {
         } else {
-          if (!m_options.m_hide_name)
+          if (ShouldShowName())
             m_stream->PutChar(' ');
           m_stream->PutCString(m_value);
           value_printed = true;
         }
       }
 
       if (m_summary.size()) {
         m_stream->Printf(" %s", m_summary.c_str());
         summary_printed = true;
       }
     }
   }
   return !error_printed;
 }
 
 bool ValueObjectPrinter::PrintObjectDescriptionIfNeeded(bool value_printed,
                                                         bool summary_printed) {
   if (ShouldPrintValueObject()) {
     // let's avoid the overly verbose no description error for a nil thing
     if (m_options.m_use_objc && !IsNil() && !IsUninitialized() &&
         (!m_options.m_pointer_as_array)) {
-      if (!m_options.m_hide_value || !m_options.m_hide_name)
+      if (!m_options.m_hide_value || ShouldShowName())
         m_stream->Printf(" ");
       const char *object_desc = nullptr;
       if (value_printed || summary_printed)
         object_desc = m_valobj->GetObjectDescription();
       else
         object_desc = GetDescriptionForDisplay();
       if (object_desc && *object_desc) {
         // If the description already ends with a \n don't add another one.
ValueObject *ValueObjectPrinter::GetValueObjectForChildrenGeneration() {
   return m_valobj;
 }
 
 void ValueObjectPrinter::PrintChildrenPreamble() {
   if (m_options.m_flat_output) {
     if (ShouldPrintValueObject())
       m_stream->EOL();
   } else {
-    if (ShouldPrintValueObject())
-      m_stream->PutCString(IsRef() ? ": {\n" : " {\n");
+    if (ShouldPrintValueObject()) {
+      if (IsRef()) {
+        m_stream->PutCString(": ");
+      } else if (ShouldShowName()) {
+        m_stream->PutChar(' ');
+      }
+      m_stream->PutCString("{\n");
+    }
     m_stream->IndentMore();
   }
 }
 
 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;
if (m_options.m_max_depth_is_default)
           .SetReachedMaximumDepth();
   } else
     m_stream->EOL();
 }
 
 bool ValueObjectPrinter::HasReachedMaximumDepth() {
   return m_curr_depth >= m_options.m_max_depth;
 }
+
+bool ValueObjectPrinter::ShouldShowName() const {
+  if (m_curr_depth == 0)
+    return !m_options.m_hide_root_name && !m_options.m_hide_name;
+  return !m_options.m_hide_name;
+}

lldb/test/API/commands/dwim-print/TestDWIMPrint.py

def test_expression_language(self):
         self._expect_cmd(f"dwim-print -l c++ -- argc + 1", "expression")
 
     def test_empty_expression(self):
         self.build()
         lldbutil.run_to_name_breakpoint(self, "main")
         error_msg = "error: 'dwim-print' takes a variable or expression"
         self.expect(f"dwim-print", error=True, startstr=error_msg)
         self.expect(f"dwim-print -- ", error=True, startstr=error_msg)
+
+    def test_nested_values(self):
+        """Test dwim-print with nested values (structs, etc)."""
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+        self.runCmd("settings set auto-one-line-summaries false")
+        self._expect_cmd(f"dwim-print s", "frame variable")
+        self._expect_cmd(f"dwim-print (struct Structure)s", "expression")

lldb/test/API/commands/dwim-print/main.c

+struct Structure {
+  int number;
+};
+
 int main(int argc, char **argv) {
+  struct Structure s;
+  s.number = 30;
+  // break here
   return 0;
 }