[Lldb-commits] [lldb] Convert ValueObject::Dump() to return llvm::Error() (NFCish) (PR #95857)

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 18 15:18:10 PDT 2024


https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/95857

>From 7e3d1420a941431de223098ee153d2d4c63cfbfc Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Mon, 17 Jun 2024 14:29:01 -0700
Subject: [PATCH 1/4] Convert ValueObject::Dump() to return llvm::Error()
 (NFCish)

This change by itself has no measurable effect on the LLDB
testsuite. I'm making it in preparation for threading through more
errors in the Swift language plugin.
---
 lldb/include/lldb/Core/ValueObject.h           |  4 ++--
 .../lldb/DataFormatters/ValueObjectPrinter.h   |  2 +-
 lldb/source/API/SBValue.cpp                    |  5 ++++-
 lldb/source/Breakpoint/Watchpoint.cpp          |  8 ++++++--
 .../source/Commands/CommandObjectDWIMPrint.cpp |  9 +++++++--
 .../Commands/CommandObjectExpression.cpp       |  6 +++++-
 lldb/source/Commands/CommandObjectFrame.cpp    | 18 +++++++++++++-----
 lldb/source/Commands/CommandObjectMemory.cpp   |  5 ++++-
 lldb/source/Commands/CommandObjectTarget.cpp   |  3 ++-
 lldb/source/Commands/CommandObjectThread.cpp   | 14 ++++++++++----
 lldb/source/Core/DumpRegisterValue.cpp         |  3 ++-
 lldb/source/Core/FormatEntity.cpp              | 11 +++++++++--
 lldb/source/Core/ValueObject.cpp               |  9 ++++++---
 .../DataFormatters/ValueObjectPrinter.cpp      | 18 +++++++++++-------
 lldb/source/Plugins/REPL/Clang/ClangREPL.cpp   |  4 +++-
 .../API/commands/dwim-print/TestDWIMPrint.py   | 16 ++++++++++++----
 lldb/test/API/commands/dwim-print/main.c       |  6 +++++-
 .../DWARF/x86/class-type-nullptr-deref.s       |  2 +-
 .../DWARF/x86/debug-types-signature-loop.s     |  2 +-
 .../DumpValueObjectOptionsTests.cpp            |  7 ++++---
 20 files changed, 108 insertions(+), 44 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index db1fdae170ed0..205d9ad9b1953 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -694,9 +694,9 @@ class ValueObject {
 
   virtual SymbolContextScope *GetSymbolContextScope();
 
-  void Dump(Stream &s);
+  llvm::Error Dump(Stream &s);
 
-  void Dump(Stream &s, const DumpValueObjectOptions &options);
+  llvm::Error Dump(Stream &s, const DumpValueObjectOptions &options);
 
   static lldb::ValueObjectSP
   CreateValueObjectFromExpression(llvm::StringRef name,
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fb5d60ba30d77..7460370c77e80 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -33,7 +33,7 @@ class ValueObjectPrinter {
 
   ~ValueObjectPrinter() = default;
 
-  bool PrintValueObject();
+  llvm::Error PrintValueObject();
 
 protected:
   typedef std::set<uint64_t> InstancePointersSet;
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..1b9cae6e350aa 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1233,7 +1233,10 @@ bool SBValue::GetDescription(SBStream &description) {
     DumpValueObjectOptions options;
     options.SetUseDynamicType(m_opaque_sp->GetUseDynamic());
     options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic());
-    value_sp->Dump(strm, options);
+    if (llvm::Error error = value_sp->Dump(strm, options)) {
+      strm << "error: " << toString(std::move(error));
+      return false;
+    }
   } else {
     strm.PutCString("No value");
   }
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index edb1a0e93460c..715e83c76697b 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -299,7 +299,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
             .SetHideRootType(true)
             .SetHideRootName(true)
             .SetHideName(true);
-        m_old_value_sp->Dump(strm, options);
+        if (llvm::Error error = m_old_value_sp->Dump(strm, options))
+          strm << "error: " << toString(std::move(error));
+
         if (strm.GetData())
           values_ss.Printf("old value: %s", strm.GetData());
       }
@@ -322,7 +324,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
             .SetHideRootType(true)
             .SetHideRootName(true)
             .SetHideName(true);
-        m_new_value_sp->Dump(strm, options);
+        if (llvm::Error error = m_new_value_sp->Dump(strm, options))
+          strm << "error: " << toString(std::move(error));
+
         if (strm.GetData())
           values_ss.Printf("new value: %s", strm.GetData());
       }
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 57a372a762e15..c1549ca6933fc 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -133,12 +133,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
   auto dump_val_object = [&](ValueObject &valobj) {
     if (is_po) {
       StreamString temp_result_stream;
-      valobj.Dump(temp_result_stream, dump_options);
+      if (llvm::Error error = valobj.Dump(temp_result_stream, dump_options)) {
+        result.AppendError(toString(std::move(error)));
+        return;
+      }
       llvm::StringRef output = temp_result_stream.GetString();
       maybe_add_hint(output);
       result.GetOutputStream() << output;
     } else {
-      valobj.Dump(result.GetOutputStream(), dump_options);
+      if (llvm::Error error =
+              valobj.Dump(result.GetOutputStream(), dump_options))
+        result.AppendError(toString(std::move(error)));
     }
   };
 
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 2319ddd3c80af..eb76753d98efc 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -461,7 +461,11 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
         options.SetVariableFormatDisplayLanguage(
             result_valobj_sp->GetPreferredDisplayLanguage());
 
-        result_valobj_sp->Dump(output_stream, options);
+        if (llvm::Error error =
+                result_valobj_sp->Dump(output_stream, options)) {
+          result.AppendError(toString(std::move(error)));
+          return false;
+        }
 
         if (suppress_result)
           if (auto result_var_sp =
diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index b1d060b3c6cf2..3f4178c1a9595 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -170,7 +170,8 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed {
     assert(valobj_sp.get() && "Must have a valid ValueObject to print");
     ValueObjectPrinter printer(*valobj_sp, &result.GetOutputStream(),
                                options);
-    printer.PrintValueObject();
+    if (llvm::Error error = printer.PrintValueObject())
+      result.AppendError(toString(std::move(error)));
   }
 
   CommandOptions m_options;
@@ -555,7 +556,9 @@ may even involve JITing and running code in the target program.)");
                                                 show_module))
                       s.PutCString(": ");
                   }
-                  valobj_sp->Dump(result.GetOutputStream(), options);
+                  auto &strm = result.GetOutputStream();
+                  if (llvm::Error error = valobj_sp->Dump(strm, options))
+                    result.AppendError(toString(std::move(error)));
                 }
               }
             } else {
@@ -597,7 +600,8 @@ may even involve JITing and running code in the target program.)");
               Stream &output_stream = result.GetOutputStream();
               options.SetRootValueObjectName(
                   valobj_sp->GetParent() ? entry.c_str() : nullptr);
-              valobj_sp->Dump(output_stream, options);
+              if (llvm::Error error = valobj_sp->Dump(output_stream, options))
+                result.AppendError(toString(std::move(error)));
             } else {
               if (auto error_cstr = error.AsCString(nullptr))
                 result.AppendError(error_cstr);
@@ -648,7 +652,9 @@ may even involve JITing and running code in the target program.)");
                     valobj_sp->GetPreferredDisplayLanguage());
                 options.SetRootValueObjectName(
                     var_sp ? var_sp->GetName().AsCString() : nullptr);
-                valobj_sp->Dump(result.GetOutputStream(), options);
+                if (llvm::Error error =
+                        valobj_sp->Dump(result.GetOutputStream(), options))
+                  result.AppendError(toString(std::move(error)));
               }
             }
           }
@@ -669,7 +675,9 @@ may even involve JITing and running code in the target program.)");
             options.SetVariableFormatDisplayLanguage(
                 rec_value_sp->GetPreferredDisplayLanguage());
             options.SetRootValueObjectName(rec_value_sp->GetName().AsCString());
-            rec_value_sp->Dump(result.GetOutputStream(), options);
+            if (llvm::Error error =
+                    rec_value_sp->Dump(result.GetOutputStream(), options))
+              result.AppendError(toString(std::move(error)));
           }
         }
       }
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 1c13484dede64..137b1ad981073 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -815,7 +815,10 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
           DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(
               eLanguageRuntimeDescriptionDisplayVerbosityFull, format));
 
-          valobj_sp->Dump(*output_stream_p, options);
+          if (llvm::Error error = valobj_sp->Dump(*output_stream_p, options)) {
+            result.AppendError(toString(std::move(error)));
+            return;
+          }
         } else {
           result.AppendErrorWithFormat(
               "failed to create a value object for: (%s) %s\n",
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index ae6c6d5479a19..80181a9b3cb71 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -784,7 +784,8 @@ class CommandObjectTargetVariable : public CommandObjectParsed {
 
     options.SetRootValueObjectName(root_name);
 
-    valobj_sp->Dump(s, options);
+    if (llvm::Error error = valobj_sp->Dump(s, options))
+      s << "error: " << toString(std::move(error));
   }
 
   static size_t GetVariableCallback(void *baton, const char *name,
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index bb2be560ebfff..5e64dd2f8f084 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1386,7 +1386,10 @@ class CommandObjectThreadException : public CommandObjectIterateOverThreads {
     Stream &strm = result.GetOutputStream();
     ValueObjectSP exception_object_sp = thread_sp->GetCurrentException();
     if (exception_object_sp) {
-      exception_object_sp->Dump(strm);
+      if (llvm::Error error = exception_object_sp->Dump(strm)) {
+        result.AppendError(toString(std::move(error)));
+        return false;
+      }
     }
 
     ThreadSP exception_thread_sp = thread_sp->GetCurrentExceptionBacktrace();
@@ -1438,9 +1441,12 @@ class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads {
       return false;
     }
     ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue();
-    if (exception_object_sp)
-      exception_object_sp->Dump(strm);
-    else
+    if (exception_object_sp) {
+      if (llvm::Error error = exception_object_sp->Dump(strm)) {
+        result.AppendError(toString(std::move(error)));
+        return false;
+      }
+    } else
       strm.Printf("(no siginfo)\n");
     strm.PutChar('\n');
 
diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp
index 463aa59267725..90b31fd0e865e 100644
--- a/lldb/source/Core/DumpRegisterValue.cpp
+++ b/lldb/source/Core/DumpRegisterValue.cpp
@@ -54,7 +54,8 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value,
       };
   dump_options.SetChildPrintingDecider(decider).SetHideRootType(true);
 
-  vobj_sp->Dump(strm, dump_options);
+  if (llvm::Error error = vobj_sp->Dump(strm, dump_options))
+    strm << "error: " << toString(std::move(error));
 }
 
 void lldb_private::DumpRegisterValue(const RegisterValue &reg_val, Stream &s,
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index 1c3a4cb1062fe..fe95858f35c9f 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1401,7 +1401,10 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
           ValueObjectSP return_valobj_sp =
               StopInfo::GetReturnValueObject(stop_info_sp);
           if (return_valobj_sp) {
-            return_valobj_sp->Dump(s);
+            if (llvm::Error error = return_valobj_sp->Dump(s)) {
+              s << "error: " << toString(std::move(error));
+              return false;
+            }
             return true;
           }
         }
@@ -1418,7 +1421,11 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
           ExpressionVariableSP expression_var_sp =
               StopInfo::GetExpressionVariable(stop_info_sp);
           if (expression_var_sp && expression_var_sp->GetValueObject()) {
-            expression_var_sp->GetValueObject()->Dump(s);
+            if (llvm::Error error =
+                    expression_var_sp->GetValueObject()->Dump(s)) {
+              s << "error: " << toString(std::move(error));
+              return false;
+            }
             return true;
           }
         }
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 1c69145560de2..cf0f557fd9129 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2713,11 +2713,14 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
   }
 }
 
-void ValueObject::Dump(Stream &s) { Dump(s, DumpValueObjectOptions(*this)); }
+llvm::Error ValueObject::Dump(Stream &s) {
+  return Dump(s, DumpValueObjectOptions(*this));
+}
 
-void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) {
+llvm::Error ValueObject::Dump(Stream &s,
+                              const DumpValueObjectOptions &options) {
   ValueObjectPrinter printer(*this, &s, options);
-  printer.PrintValueObject();
+  return printer.PrintValueObject();
 }
 
 ValueObjectSP ValueObject::CreateConstantValue(ConstString name) {
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index c2933d8574583..a81a51720a495 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -69,15 +69,13 @@ void ValueObjectPrinter::Init(
   SetupMostSpecializedValue();
 }
 
-bool ValueObjectPrinter::PrintValueObject() {
+llvm::Error ValueObjectPrinter::PrintValueObject() {
   // If the incoming ValueObject is in an error state, the best we're going to 
   // get out of it is its type.  But if we don't even have that, just print
   // the error and exit early.
   if (m_orig_valobj.GetError().Fail() &&
-      !m_orig_valobj.GetCompilerType().IsValid()) {
-    m_stream->Printf("Error: '%s'", m_orig_valobj.GetError().AsCString());
-    return true;
-  }
+      !m_orig_valobj.GetCompilerType().IsValid())
+    return m_orig_valobj.GetError().ToError();
 
   if (ShouldPrintValueObject()) {
     PrintLocationIfNeeded();
@@ -97,7 +95,7 @@ bool ValueObjectPrinter::PrintValueObject() {
   else
     m_stream->EOL();
 
-  return true;
+  return llvm::Error::success();
 }
 
 ValueObject &ValueObjectPrinter::GetMostSpecializedValue() {
@@ -619,7 +617,13 @@ void ValueObjectPrinter::PrintChild(
     ValueObjectPrinter child_printer(*(child_sp.get()), m_stream, child_options,
                                      ptr_depth, m_curr_depth + 1,
                                      m_printed_instance_pointers);
-    child_printer.PrintValueObject();
+    llvm::Error error = child_printer.PrintValueObject();
+    if (error) {
+      if (m_stream)
+        *m_stream << "error: " << toString(std::move(error));
+      else
+        llvm::consumeError(std::move(error));
+    }
   }
 }
 
diff --git a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
index 0aaddad53126e..0fb5490defc63 100644
--- a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
+++ b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
@@ -95,7 +95,9 @@ bool ClangREPL::PrintOneVariable(Debugger &debugger,
     if (m_implicit_expr_result_regex.Execute(var->GetName().GetStringRef()))
       return true;
   }
-  valobj_sp->Dump(*output_sp);
+  if (llvm::Error error = valobj_sp->Dump(*output_sp))
+    *output_sp << "error: " << toString(std::move(error));
+
   return true;
 }
 
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index c650b1e3533e0..785a9621c99dc 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -122,7 +122,7 @@ 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, "break here", lldb.SBFileSpec("main.c")
         )
         self.runCmd("settings set auto-one-line-summaries false")
         self._expect_cmd(f"dwim-print s", "frame variable")
@@ -132,7 +132,7 @@ def test_summary_strings(self):
         """Test dwim-print with nested values (structs, etc)."""
         self.build()
         lldbutil.run_to_source_breakpoint(
-            self, "// break here", lldb.SBFileSpec("main.c")
+            self, "break here", lldb.SBFileSpec("main.c")
         )
         self.runCmd("settings set auto-one-line-summaries false")
         self.runCmd("type summary add -e -s 'stub summary' Structure")
@@ -143,7 +143,7 @@ def test_void_result(self):
         """Test dwim-print does not surface an error message for void expressions."""
         self.build()
         lldbutil.run_to_source_breakpoint(
-            self, "// break here", lldb.SBFileSpec("main.c")
+            self, "break here", lldb.SBFileSpec("main.c")
         )
         self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
 
@@ -151,10 +151,18 @@ def test_preserves_persistent_variables(self):
         """Test dwim-print does not delete persistent variables."""
         self.build()
         lldbutil.run_to_source_breakpoint(
-            self, "// break here", lldb.SBFileSpec("main.c")
+            self, "break here", lldb.SBFileSpec("main.c")
         )
         self.expect("dwim-print int $i = 15")
         # Run the same expression twice and verify success. This ensures the
         # first command does not delete the persistent variable.
         for _ in range(2):
             self.expect("dwim-print $i", startstr="(int) 15")
+
+    def test_missing_type(self):
+        """The expected output of po opaque is its address (no error)"""
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.c")
+        )
+        self.expect("dwim-print -O -- opaque", substrs=['0x'])
diff --git a/lldb/test/API/commands/dwim-print/main.c b/lldb/test/API/commands/dwim-print/main.c
index e2eccf3a88b4b..6bfe645c7c6e0 100644
--- a/lldb/test/API/commands/dwim-print/main.c
+++ b/lldb/test/API/commands/dwim-print/main.c
@@ -2,9 +2,13 @@ struct Structure {
   int number;
 };
 
+struct Opaque;
+int puts(const char *s);
+
 int main(int argc, char **argv) {
   struct Structure s;
   s.number = 30;
-  // break here
+  struct Opaque *opaque = &s;
+  puts("break here");
   return 0;
 }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
index c7aea06bf9098..4fbc1894c8c44 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -4,7 +4,7 @@
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
 # RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
 
-# CHECK: 'Unable to determine byte size.'
+# CHECK: Unable to determine byte size.
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s
index 64b835353ed4e..64b22830e8f28 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s
@@ -4,7 +4,7 @@
 # RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -o "target variable e" -b | FileCheck %s
 
-# CHECK: Error: 'Unable to determine byte size.'
+# CHECK: error: Unable to determine byte size.
 
         .type   e, at object               # @e
         .section        .rodata,"a", at progbits
diff --git a/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp b/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp
index 31068a04d8dfe..6cb982d7f5980 100644
--- a/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp
+++ b/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp
@@ -98,9 +98,10 @@ class ValueObjectMockProcessTest : public ::testing::Test {
     ExecutionContextScope *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
     for (auto [value, options, expected] : tests) {
       DataExtractor data_extractor{&value, sizeof(value), endian, 4};
-      ValueObjectConstResult::Create(exe_scope, enum_type, var_name,
-                                     data_extractor)
-          ->Dump(strm, options);
+      auto valobj_sp = ValueObjectConstResult::Create(exe_scope, enum_type,
+                                                      var_name, data_extractor);
+      if (llvm::Error error = valobj_sp->Dump(strm, options))
+        llvm::consumeError(std::move(error));
       ASSERT_STREQ(strm.GetString().str().c_str(), expected);
       strm.Clear();
     }

>From 891a1a3982ef2c4d7ad899d691c42d23864de467 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Mon, 17 Jun 2024 17:07:36 -0700
Subject: [PATCH 2/4] Refactor GetObjectDescription() to return llvm::Expected
 (NFC)

This is de facto an NFC change for Objective-C but will benefit the
Swift language plugin.
---
 lldb/include/lldb/Core/ValueObject.h          |  2 +-
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  7 +-
 lldb/include/lldb/Target/LanguageRuntime.h    |  9 ++-
 lldb/source/API/SBValue.cpp                   |  5 +-
 lldb/source/Core/ValueObject.cpp              | 44 +++++++----
 .../DataFormatters/ValueObjectPrinter.cpp     | 73 +++++++++++--------
 .../CPlusPlus/CPPLanguageRuntime.cpp          | 13 ++--
 .../CPlusPlus/CPPLanguageRuntime.h            |  6 +-
 .../AppleObjCRuntime/AppleObjCRuntime.cpp     | 57 ++++++++-------
 .../ObjC/AppleObjCRuntime/AppleObjCRuntime.h  |  6 +-
 .../GNUstepObjCRuntime/GNUstepObjCRuntime.cpp | 15 ++--
 .../GNUstepObjCRuntime/GNUstepObjCRuntime.h   |  7 +-
 12 files changed, 142 insertions(+), 102 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index 205d9ad9b1953..93eb3e8f590f4 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -537,7 +537,7 @@ class ValueObject {
                            std::string &destination,
                            const TypeSummaryOptions &options);
 
-  const char *GetObjectDescription();
+  llvm::Expected<std::string> GetObjectDescription();
 
   bool HasSpecialPrintableRepresentation(
       ValueObjectRepresentationStyle val_obj_display,
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index 7460370c77e80..f9deb6ebf8d6d 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -75,7 +75,7 @@ class ValueObjectPrinter {
 
   void SetupMostSpecializedValue();
 
-  const char *GetDescriptionForDisplay();
+  llvm::Expected<std::string> GetDescriptionForDisplay();
 
   const char *GetRootNameForDisplay();
 
@@ -108,7 +108,8 @@ class ValueObjectPrinter {
 
   bool PrintValueAndSummaryIfNeeded(bool &value_printed, bool &summary_printed);
 
-  bool PrintObjectDescriptionIfNeeded(bool value_printed, bool summary_printed);
+  llvm::Error PrintObjectDescriptionIfNeeded(bool value_printed,
+                                             bool summary_printed);
 
   bool
   ShouldPrintChildren(DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
@@ -132,7 +133,7 @@ class ValueObjectPrinter {
   PrintChildren(bool value_printed, bool summary_printed,
                 const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
-  void PrintChildrenIfNeeded(bool value_printed, bool summary_printed);
+  llvm::Error PrintChildrenIfNeeded(bool value_printed, bool summary_printed);
 
   bool PrintChildrenOneLiner(bool hide_names);
 
diff --git a/lldb/include/lldb/Target/LanguageRuntime.h b/lldb/include/lldb/Target/LanguageRuntime.h
index a2a9c0163f082..d093dccfeae85 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -73,11 +73,12 @@ class LanguageRuntime : public Runtime, public PluginInterface {
     return nullptr;
   }
 
-  virtual bool GetObjectDescription(Stream &str, ValueObject &object) = 0;
-
-  virtual bool GetObjectDescription(Stream &str, Value &value,
-                                    ExecutionContextScope *exe_scope) = 0;
+  virtual llvm::Error GetObjectDescription(Stream &str,
+                                           ValueObject &object) = 0;
 
+  virtual llvm::Error
+  GetObjectDescription(Stream &str, Value &value,
+                       ExecutionContextScope *exe_scope) = 0;
 
   struct VTableInfo {
     Address addr; /// Address of the vtable's virtual function table
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 1b9cae6e350aa..10a691c403419 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -379,7 +379,10 @@ const char *SBValue::GetObjectDescription() {
   if (!value_sp)
     return nullptr;
 
-  return ConstString(value_sp->GetObjectDescription()).GetCString();
+  llvm::Expected<std::string> str = value_sp->GetObjectDescription();
+  if (!str)
+    return ConstString("error: " + toString(str.takeError())).AsCString();
+  return ConstString(*str).AsCString();
 }
 
 SBType SBValue::GetType() {
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index cf0f557fd9129..8f72efc2299b4 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -989,41 +989,46 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP &buffer_sp,
   return {total_bytes_read, was_capped};
 }
 
-const char *ValueObject::GetObjectDescription() {
+llvm::Expected<std::string> ValueObject::GetObjectDescription() {
   if (!UpdateValueIfNeeded(true))
-    return nullptr;
+    return llvm::createStringError("could not update value");
 
   // Return cached value.
   if (!m_object_desc_str.empty())
-    return m_object_desc_str.c_str();
+    return m_object_desc_str;
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   Process *process = exe_ctx.GetProcessPtr();
   if (!process)
-    return nullptr;
+    return llvm::createStringError("no process");
 
   // Returns the object description produced by one language runtime.
-  auto get_object_description = [&](LanguageType language) -> const char * {
+  auto get_object_description =
+      [&](LanguageType language) -> llvm::Expected<std::string> {
     if (LanguageRuntime *runtime = process->GetLanguageRuntime(language)) {
       StreamString s;
-      if (runtime->GetObjectDescription(s, *this)) {
-        m_object_desc_str.append(std::string(s.GetString()));
-        return m_object_desc_str.c_str();
-      }
+      if (llvm::Error error = runtime->GetObjectDescription(s, *this))
+        return error;
+      m_object_desc_str = s.GetString();
+      return m_object_desc_str;
     }
-    return nullptr;
+    return llvm::createStringError("no native language runtime");
   };
 
   // Try the native language runtime first.
   LanguageType native_language = GetObjectRuntimeLanguage();
-  if (const char *desc = get_object_description(native_language))
+  llvm::Expected<std::string> desc = get_object_description(native_language);
+  if (desc)
     return desc;
 
   // Try the Objective-C language runtime. This fallback is necessary
   // for Objective-C++ and mixed Objective-C / C++ programs.
-  if (Language::LanguageIsCFamily(native_language))
+  if (Language::LanguageIsCFamily(native_language)) {
+    // We're going to try again, so let's drop the first error.
+    llvm::consumeError(desc.takeError());
     return get_object_description(eLanguageTypeObjC);
-  return nullptr;
+  }
+  return desc;
 }
 
 bool ValueObject::GetValueAsCString(const lldb_private::TypeFormatImpl &format,
@@ -1472,9 +1477,16 @@ bool ValueObject::DumpPrintableRepresentation(
       str = GetSummaryAsCString();
       break;
 
-    case eValueObjectRepresentationStyleLanguageSpecific:
-      str = GetObjectDescription();
-      break;
+    case eValueObjectRepresentationStyleLanguageSpecific: {
+      llvm::Expected<std::string> desc = GetObjectDescription();
+      if (!desc) {
+        strm << "error: " << toString(desc.takeError());
+        str = strm.GetString();
+      } else {
+        strm << *desc;
+        str = strm.GetString();
+      }
+    } break;
 
     case eValueObjectRepresentationStyleLocation:
       str = GetLocationAsCString();
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index a81a51720a495..ce24a7866e53a 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -91,9 +91,8 @@ llvm::Error ValueObjectPrinter::PrintValueObject() {
       PrintValueAndSummaryIfNeeded(value_printed, summary_printed);
 
   if (m_val_summary_ok)
-    PrintChildrenIfNeeded(value_printed, summary_printed);
-  else
-    m_stream->EOL();
+    return PrintChildrenIfNeeded(value_printed, summary_printed);
+  m_stream->EOL();
 
   return llvm::Error::success();
 }
@@ -145,13 +144,21 @@ void ValueObjectPrinter::SetupMostSpecializedValue() {
          "SetupMostSpecialized value must compute a valid ValueObject");
 }
 
-const char *ValueObjectPrinter::GetDescriptionForDisplay() {
+llvm::Expected<std::string> ValueObjectPrinter::GetDescriptionForDisplay() {
   ValueObject &valobj = GetMostSpecializedValue();
-  const char *str = valobj.GetObjectDescription();
+  llvm::Expected<std::string> maybe_str = valobj.GetObjectDescription();
+  if (maybe_str)
+    return maybe_str;
+
+  const char *str = nullptr;
   if (!str)
     str = valobj.GetSummaryAsCString();
   if (!str)
     str = valobj.GetValueAsCString();
+
+  if (!str)
+    return maybe_str;
+  llvm::consumeError(maybe_str.takeError());
   return str;
 }
 
@@ -461,34 +468,38 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed,
   return !error_printed;
 }
 
-bool ValueObjectPrinter::PrintObjectDescriptionIfNeeded(bool value_printed,
-                                                        bool summary_printed) {
+llvm::Error
+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 || ShouldShowName())
-        m_stream->Printf(" ");
-      const char *object_desc = nullptr;
-      if (value_printed || summary_printed)
-        object_desc = GetMostSpecializedValue().GetObjectDescription();
-      else
-        object_desc = GetDescriptionForDisplay();
-      if (object_desc && *object_desc) {
+        *m_stream << ' ';
+      llvm::Expected<std::string> object_desc =
+          (value_printed || summary_printed)
+              ? GetMostSpecializedValue().GetObjectDescription()
+              : GetDescriptionForDisplay();
+      if (!object_desc) {
+        // If no value or summary was printed, surface the error.
+        if (!value_printed && !summary_printed)
+          return object_desc.takeError();
+        // Otherwise gently nudge the user that they should have used
+        // `p` instead of `po`. Unfortunately we cannot be more direct
+        // about this, because we don't actually know what the user did.
+        *m_stream << "warning: no object description available\n";
+        llvm::consumeError(object_desc.takeError());
+      } else {
+        *m_stream << *object_desc;
         // If the description already ends with a \n don't add another one.
-        size_t object_end = strlen(object_desc) - 1;
-        if (object_desc[object_end] == '\n')
-          m_stream->Printf("%s", object_desc);
-        else
-          m_stream->Printf("%s\n", object_desc);
-        return true;
-      } else if (!value_printed && !summary_printed)
-        return true;
-      else
-        return false;
+        if (object_desc->empty() || object_desc->back() != '\n')
+          *m_stream << '\n';
+      }
+      return llvm::Error::success();
     }
   }
-  return true;
+  return llvm::Error::success();
 }
 
 bool DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const {
@@ -812,9 +823,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) {
   return true;
 }
 
-void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
-                                               bool summary_printed) {
-  PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
+llvm::Error ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
+                                                      bool summary_printed) {
+  auto error = PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
+  if (error)
+    return error;
+
   ValueObject &valobj = GetMostSpecializedValue();
 
   DumpValueObjectOptions::PointerDepth curr_ptr_depth = m_ptr_depth;
@@ -830,7 +844,7 @@ void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
     if (m_printed_instance_pointers->count(instance_ptr_value)) {
       // We already printed this instance-is-pointer thing, so don't expand it.
       m_stream->PutCString(" {...}\n");
-      return;
+      return llvm::Error::success();
     } else {
       // Remember this guy for future reference.
       m_printed_instance_pointers->emplace(instance_ptr_value);
@@ -858,6 +872,7 @@ void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
           .SetReachedMaximumDepth();
   } else
     m_stream->EOL();
+  return llvm::Error::success();
 }
 
 bool ValueObjectPrinter::HasReachedMaximumDepth() {
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index 300ecc8e8ed58..c7202a47d0157 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -47,16 +47,17 @@ bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
   return name == g_this || name == g_promise || name == g_coro_frame;
 }
 
-bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
-                                              ValueObject &object) {
+llvm::Error CPPLanguageRuntime::GetObjectDescription(Stream &str,
+                                                     ValueObject &object) {
   // C++ has no generic way to do this.
-  return false;
+  return llvm::createStringError("C++ does not support object descriptions");
 }
 
-bool CPPLanguageRuntime::GetObjectDescription(
-    Stream &str, Value &value, ExecutionContextScope *exe_scope) {
+llvm::Error
+CPPLanguageRuntime::GetObjectDescription(Stream &str, Value &value,
+                                         ExecutionContextScope *exe_scope) {
   // C++ has no generic way to do this.
-  return false;
+  return llvm::createStringError("C++ does not support object descriptions");
 }
 
 bool contains_lambda_identifier(llvm::StringRef &str_ref) {
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
index 1be58b7bf9ea9..57cfe28245808 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -59,10 +59,10 @@ class CPPLanguageRuntime : public LanguageRuntime {
         process.GetLanguageRuntime(lldb::eLanguageTypeC_plus_plus));
   }
 
-  bool GetObjectDescription(Stream &str, ValueObject &object) override;
+  llvm::Error GetObjectDescription(Stream &str, ValueObject &object) override;
 
-  bool GetObjectDescription(Stream &str, Value &value,
-                            ExecutionContextScope *exe_scope) override;
+  llvm::Error GetObjectDescription(Stream &str, Value &value,
+                                   ExecutionContextScope *exe_scope) override;
 
   /// Obtain a ThreadPlan to get us into C++ constructs such as std::function.
   ///
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 9434376f7d9ea..800eda925591e 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -67,20 +67,21 @@ void AppleObjCRuntime::Terminate() {
   AppleObjCRuntimeV1::Terminate();
 }
 
-bool AppleObjCRuntime::GetObjectDescription(Stream &str, ValueObject &valobj) {
+llvm::Error AppleObjCRuntime::GetObjectDescription(Stream &str,
+                                                   ValueObject &valobj) {
   CompilerType compiler_type(valobj.GetCompilerType());
   bool is_signed;
   // ObjC objects can only be pointers (or numbers that actually represents
   // pointers but haven't been typecast, because reasons..)
   if (!compiler_type.IsIntegerType(is_signed) && !compiler_type.IsPointerType())
-    return false;
+    return llvm::createStringError("not a pointer type");
 
   // Make the argument list: we pass one arg, the address of our pointer, to
   // the print function.
   Value val;
 
   if (!valobj.ResolveValue(val.GetScalar()))
-    return false;
+    return llvm::createStringError("pointer value could not be resolved");
 
   // Value Objects may not have a process in their ExecutionContextRef.  But we
   // need to have one in the ref we pass down to eventually call description.
@@ -91,20 +92,22 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &str, ValueObject &valobj) {
   } else {
     exe_ctx.SetContext(valobj.GetTargetSP(), true);
     if (!exe_ctx.HasProcessScope())
-      return false;
+      return llvm::createStringError("no process");
   }
   return GetObjectDescription(str, val, exe_ctx.GetBestExecutionContextScope());
 }
-bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
-                                            ExecutionContextScope *exe_scope) {
+
+llvm::Error
+AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
+                                       ExecutionContextScope *exe_scope) {
   if (!m_read_objc_library)
-    return false;
+    return llvm::createStringError("Objective-C runtime not loaded");
 
   ExecutionContext exe_ctx;
   exe_scope->CalculateExecutionContext(exe_ctx);
   Process *process = exe_ctx.GetProcessPtr();
   if (!process)
-    return false;
+    return llvm::createStringError("no process");
 
   // We need other parts of the exe_ctx, but the processes have to match.
   assert(m_process == process);
@@ -112,25 +115,25 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
   // Get the function address for the print function.
   const Address *function_address = GetPrintForDebuggerAddr();
   if (!function_address)
-    return false;
+    return llvm::createStringError("no print function");
 
   Target *target = exe_ctx.GetTargetPtr();
   CompilerType compiler_type = value.GetCompilerType();
   if (compiler_type) {
-    if (!TypeSystemClang::IsObjCObjectPointerType(compiler_type)) {
-      strm.Printf("Value doesn't point to an ObjC object.\n");
-      return false;
-    }
+    if (!TypeSystemClang::IsObjCObjectPointerType(compiler_type))
+      return llvm::createStringError(
+          "Value doesn't point to an ObjC object.\n");
   } else {
     // If it is not a pointer, see if we can make it into a pointer.
     TypeSystemClangSP scratch_ts_sp =
         ScratchTypeSystemClang::GetForTarget(*target);
     if (!scratch_ts_sp)
-      return false;
+      return llvm::createStringError("no scratch type system");
 
     CompilerType opaque_type = scratch_ts_sp->GetBasicType(eBasicTypeObjCID);
     if (!opaque_type)
-      opaque_type = scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
+      opaque_type =
+          scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
     // value.SetContext(Value::eContextTypeClangType, opaque_type_ptr);
     value.SetCompilerType(opaque_type);
   }
@@ -142,14 +145,14 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
   TypeSystemClangSP scratch_ts_sp =
       ScratchTypeSystemClang::GetForTarget(*target);
   if (!scratch_ts_sp)
-    return false;
+    return llvm::createStringError("no scratch type system");
 
   CompilerType return_compiler_type = scratch_ts_sp->GetCStringType(true);
   Value ret;
   //    ret.SetContext(Value::eContextTypeClangType, return_compiler_type);
   ret.SetCompilerType(return_compiler_type);
 
-  if (exe_ctx.GetFramePtr() == nullptr) {
+  if (!exe_ctx.GetFramePtr()) {
     Thread *thread = exe_ctx.GetThreadPtr();
     if (thread == nullptr) {
       exe_ctx.SetThreadSP(process->GetThreadList().GetSelectedThread());
@@ -173,10 +176,11 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
             arg_value_list, "objc-object-description", error));
     if (error.Fail()) {
       m_print_object_caller_up.reset();
-      strm.Printf("Could not get function runner to call print for debugger "
-                  "function: %s.",
-                  error.AsCString());
-      return false;
+      return llvm::createStringError(
+          llvm::Twine(
+              "could not get function runner to call print for debugger "
+              "function: ") +
+          error.AsCString());
     }
     m_print_object_caller_up->InsertFunction(exe_ctx, wrapper_struct_addr,
                                              diagnostics);
@@ -195,10 +199,9 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
 
   ExpressionResults results = m_print_object_caller_up->ExecuteFunction(
       exe_ctx, &wrapper_struct_addr, options, diagnostics, ret);
-  if (results != eExpressionCompleted) {
-    strm.Printf("Error evaluating Print Object function: %d.\n", results);
-    return false;
-  }
+  if (results != eExpressionCompleted)
+    return llvm::createStringError(
+        "Error evaluating Print Object function: %d.\n", results);
 
   addr_t result_ptr = ret.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 
@@ -213,7 +216,9 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
     strm.Write(buf, curr_len);
     cstr_len += curr_len;
   }
-  return cstr_len > 0;
+  if (cstr_len > 0)
+    return llvm::Error::success();
+  return llvm::createStringError("empty object description");
 }
 
 lldb::ModuleSP AppleObjCRuntime::GetObjCModule() {
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h
index cfeec3d679bef..da58d44db19a8 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h
@@ -44,10 +44,10 @@ class AppleObjCRuntime : public lldb_private::ObjCLanguageRuntime {
   }
 
   // These are generic runtime functions:
-  bool GetObjectDescription(Stream &str, Value &value,
-                            ExecutionContextScope *exe_scope) override;
+  llvm::Error GetObjectDescription(Stream &str, Value &value,
+                                   ExecutionContextScope *exe_scope) override;
 
-  bool GetObjectDescription(Stream &str, ValueObject &object) override;
+  llvm::Error GetObjectDescription(Stream &str, ValueObject &object) override;
 
   bool CouldHaveDynamicValue(ValueObject &in_value) override;
 
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
index 39b3e816f4bec..5e1face756d5f 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
@@ -104,15 +104,16 @@ GNUstepObjCRuntime::GNUstepObjCRuntime(Process *process)
   ReadObjCLibraryIfNeeded(process->GetTarget().GetImages());
 }
 
-bool GNUstepObjCRuntime::GetObjectDescription(Stream &str,
-                                              ValueObject &valobj) {
-  // TODO: ObjC has a generic way to do this
-  return false;
+llvm::Error GNUstepObjCRuntime::GetObjectDescription(Stream &str,
+                                                     ValueObject &valobj) {
+  return llvm::createStringError(
+      "LLDB's GNUStep runtime does not support object description");
 }
-bool GNUstepObjCRuntime::GetObjectDescription(
+
+llvm::Error GNUstepObjCRuntime::GetObjectDescription(
     Stream &strm, Value &value, ExecutionContextScope *exe_scope) {
-  // TODO: ObjC has a generic way to do this
-  return false;
+  return llvm::createStringError(
+      "LLDB's GNUStep runtime does not support object description");
 }
 
 bool GNUstepObjCRuntime::CouldHaveDynamicValue(ValueObject &in_value) {
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h
index b22088807f97d..f45b7ebb0b99e 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h
@@ -57,10 +57,11 @@ class GNUstepObjCRuntime : public lldb_private::ObjCLanguageRuntime {
   //
   // LanguageRuntime implementation
   //
-  bool GetObjectDescription(Stream &str, Value &value,
-                            ExecutionContextScope *exe_scope) override;
+  llvm::Error
+  GetObjectDescription(Stream &str, Value &value,
+                       ExecutionContextScope *exe_scope) override;
 
-  bool GetObjectDescription(Stream &str, ValueObject &object) override;
+  llvm::Error GetObjectDescription(Stream &str, ValueObject &object) override;
 
   bool CouldHaveDynamicValue(ValueObject &in_value) override;
 

>From 6e21ae786635b0bd4abe5e4973ab5a994d74bf23 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Tue, 18 Jun 2024 12:19:28 -0700
Subject: [PATCH 3/4] Factor out expression result error strings.

---
 .../source/Interpreter/CommandInterpreter.cpp | 48 ++-----------------
 .../AppleObjCRuntime/AppleObjCRuntime.cpp     |  3 +-
 lldb/source/Utility/CMakeLists.txt            |  1 +
 3 files changed, 6 insertions(+), 46 deletions(-)

diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index da995de1407c4..40c915b2c94fc 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -48,6 +48,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/StreamFile.h"
+#include "lldb/Utility/ErrorMessages.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -1817,51 +1818,8 @@ CommandInterpreter::PreprocessToken(std::string &expr_str) {
     error = expr_result_valobj_sp->GetError();
 
   if (error.Success()) {
-    switch (expr_result) {
-    case eExpressionSetupError:
-      error.SetErrorStringWithFormat(
-          "expression setup error for the expression '%s'", expr_str.c_str());
-      break;
-    case eExpressionParseError:
-      error.SetErrorStringWithFormat(
-          "expression parse error for the expression '%s'", expr_str.c_str());
-      break;
-    case eExpressionResultUnavailable:
-      error.SetErrorStringWithFormat(
-          "expression error fetching result for the expression '%s'",
-          expr_str.c_str());
-      break;
-    case eExpressionCompleted:
-      break;
-    case eExpressionDiscarded:
-      error.SetErrorStringWithFormat(
-          "expression discarded for the expression '%s'", expr_str.c_str());
-      break;
-    case eExpressionInterrupted:
-      error.SetErrorStringWithFormat(
-          "expression interrupted for the expression '%s'", expr_str.c_str());
-      break;
-    case eExpressionHitBreakpoint:
-      error.SetErrorStringWithFormat(
-          "expression hit breakpoint for the expression '%s'",
-          expr_str.c_str());
-      break;
-    case eExpressionTimedOut:
-      error.SetErrorStringWithFormat(
-          "expression timed out for the expression '%s'", expr_str.c_str());
-      break;
-    case eExpressionStoppedForDebug:
-      error.SetErrorStringWithFormat("expression stop at entry point "
-                                     "for debugging for the "
-                                     "expression '%s'",
-                                     expr_str.c_str());
-      break;
-    case eExpressionThreadVanished:
-      error.SetErrorStringWithFormat(
-          "expression thread vanished for the expression '%s'",
-          expr_str.c_str());
-      break;
-    }
+    std::string result = lldb_private::toString(expr_result);
+    error.SetErrorString(result + "for the expression '" + expr_str + "'");
   }
   return error;
 }
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 800eda925591e..5ff267720629e 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -31,6 +31,7 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/ErrorMessages.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Scalar.h"
@@ -201,7 +202,7 @@ AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
       exe_ctx, &wrapper_struct_addr, options, diagnostics, ret);
   if (results != eExpressionCompleted)
     return llvm::createStringError(
-        "Error evaluating Print Object function: %d.\n", results);
+        "could not evaluate print object function: " + toString(results));
 
   addr_t result_ptr = ret.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
 
diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt
index a3b0a405b4133..e9954d66cd1a5 100644
--- a/lldb/source/Utility/CMakeLists.txt
+++ b/lldb/source/Utility/CMakeLists.txt
@@ -39,6 +39,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES
   DataExtractor.cpp
   Diagnostics.cpp
   Environment.cpp
+  ErrorMessages.cpp
   Event.cpp
   FileSpec.cpp
   FileSpecList.cpp

>From b1fd65f9313d5c462ea9ae8f8ac5f6275334d503 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Tue, 18 Jun 2024 13:44:45 -0700
Subject: [PATCH 4/4] Reformat test (NFC)

---
 .../API/commands/dwim-print/TestDWIMPrint.py     | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 785a9621c99dc..fb91c9997b717 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -121,9 +121,7 @@ def test_empty_expression(self):
     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")
-        )
+        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")
@@ -131,9 +129,7 @@ def test_nested_values(self):
     def test_summary_strings(self):
         """Test dwim-print with nested values (structs, etc)."""
         self.build()
-        lldbutil.run_to_source_breakpoint(
-            self, "break here", lldb.SBFileSpec("main.c")
-        )
+        lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
         self.runCmd("settings set auto-one-line-summaries false")
         self.runCmd("type summary add -e -s 'stub summary' Structure")
         self._expect_cmd(f"dwim-print s", "frame variable")
@@ -142,17 +138,13 @@ def test_summary_strings(self):
     def test_void_result(self):
         """Test dwim-print does not surface an error message for void expressions."""
         self.build()
-        lldbutil.run_to_source_breakpoint(
-            self, "break here", lldb.SBFileSpec("main.c")
-        )
+        lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
         self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"])
 
     def test_preserves_persistent_variables(self):
         """Test dwim-print does not delete persistent variables."""
         self.build()
-        lldbutil.run_to_source_breakpoint(
-            self, "break here", lldb.SBFileSpec("main.c")
-        )
+        lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
         self.expect("dwim-print int $i = 15")
         # Run the same expression twice and verify success. This ensures the
         # first command does not delete the persistent variable.



More information about the lldb-commits mailing list