https://github.com/kastiglione updated 
https://github.com/llvm/llvm-project/pull/152417

>From 93c8135a07e21ef0f5764735cd6ddc97c9b48a32 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Wed, 6 Aug 2025 17:13:02 -0700
Subject: [PATCH 1/6] [lldb] Propagate ExpressionErrors from
 ValueObjectPrinter::GetDescriptionForDisplay

---
 lldb/source/DataFormatters/ValueObjectPrinter.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index c2f8bb3ea05bc..c5a6de2de35f2 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -9,6 +9,7 @@
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
 
 #include "lldb/DataFormatters/DataVisualization.h"
+#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
@@ -150,6 +151,11 @@ llvm::Expected<std::string> 
ValueObjectPrinter::GetDescriptionForDisplay() {
   if (maybe_str)
     return maybe_str;
 
+  if (maybe_str.errorIsA<lldb_private::ExpressionError>())
+    // Propagate expression errors to expose diagnostics to the user.
+    // Without this early exit, the summary/value may be shown without errors.
+    return maybe_str;
+
   const char *str = nullptr;
   if (!str)
     str = valobj.GetSummaryAsCString();

>From ce204ae099917a229cd8c3b9579ddcfd0b74f5eb Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Thu, 14 Aug 2025 15:00:04 -0700
Subject: [PATCH 2/6] Rework handling of po failure

---
 .../DataFormatters/DumpValueObjectOptions.h   |   2 +
 .../lldb/DataFormatters/ValueObjectPrinter.h  |   6 +-
 .../DataFormatters/DumpValueObjectOptions.cpp |  10 ++
 .../DataFormatters/ValueObjectPrinter.cpp     | 103 ++++++++----------
 4 files changed, 58 insertions(+), 63 deletions(-)

diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h 
b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
index cdb620e2148de..c61d6d1a418ff 100644
--- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
+++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
@@ -76,6 +76,8 @@ class DumpValueObjectOptions {
 
   DumpValueObjectOptions &SetShowLocation(bool show = false);
 
+  DumpValueObjectOptions &DisableObjectiveC();
+
   DumpValueObjectOptions &SetUseObjectiveC(bool use = false);
 
   DumpValueObjectOptions &SetShowSummary(bool show = true);
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index f9deb6ebf8d6d..fdea3682f8844 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -75,8 +75,6 @@ class ValueObjectPrinter {
 
   void SetupMostSpecializedValue();
 
-  llvm::Expected<std::string> GetDescriptionForDisplay();
-
   const char *GetRootNameForDisplay();
 
   bool ShouldPrintValueObject();
@@ -108,8 +106,7 @@ class ValueObjectPrinter {
 
   bool PrintValueAndSummaryIfNeeded(bool &value_printed, bool 
&summary_printed);
 
-  llvm::Error PrintObjectDescriptionIfNeeded(bool value_printed,
-                                             bool summary_printed);
+  void PrintObjectDescriptionIfNeeded(std::optional<std::string> object_desc);
 
   bool
   ShouldPrintChildren(DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
@@ -141,6 +138,7 @@ class ValueObjectPrinter {
 
 private:
   bool ShouldShowName() const;
+  bool ShouldPrintObjectDescription();
 
   ValueObject &m_orig_valobj;
   /// Cache the current "most specialized" value.  Don't use this
diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp 
b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
index c343e6083df19..8ce53020e8195 100644
--- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
+++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
@@ -65,6 +65,16 @@ DumpValueObjectOptions 
&DumpValueObjectOptions::SetShowLocation(bool show) {
   return *this;
 }
 
+DumpValueObjectOptions &DumpValueObjectOptions::DisableObjectiveC() {
+  // Reset these options to their default values.
+  SetUseObjectiveC(false);
+  SetHideRootType(false);
+  SetHideRootName(false);
+  SetHideValue(false);
+  SetShowSummary(true);
+  return *this;
+}
+
 DumpValueObjectOptions &DumpValueObjectOptions::SetUseObjectiveC(bool use) {
   m_use_objc = use;
   return *this;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index c5a6de2de35f2..4dde8bcde3a4c 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -15,9 +15,11 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/ValueObject/ValueObject.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MathExtras.h"
 #include <cstdint>
 #include <memory>
+#include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -70,6 +72,18 @@ void ValueObjectPrinter::Init(
   SetupMostSpecializedValue();
 }
 
+static const char *maybeNewline(const std::string &s) {
+  // If the string already ends with a \n don't add another one.
+  if (s.empty() || s.back() != '\n')
+    return "\n";
+  return "";
+}
+
+bool ValueObjectPrinter::ShouldPrintObjectDescription() {
+  return ShouldPrintValueObject() && m_options.m_use_objc && !IsNil() &&
+         !IsUninitialized() && !m_options.m_pointer_as_array;
+}
+
 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
@@ -78,6 +92,25 @@ llvm::Error ValueObjectPrinter::PrintValueObject() {
       !m_orig_valobj.GetCompilerType().IsValid())
     return m_orig_valobj.GetError().ToError();
 
+  std::optional<std::string> object_desc;
+  if (ShouldPrintObjectDescription()) {
+    // The object description is invoked now, but not printed until after
+    // value/summary. Calling GetObjectDescription at the outset of printing
+    // allows for early discovery of errors. In the case of an error, the value
+    // object is printed normally.
+    llvm::Expected<std::string> object_desc_or_err =
+        GetMostSpecializedValue().GetObjectDescription();
+    if (!object_desc_or_err) {
+      auto error_msg = toString(object_desc_or_err.takeError());
+      *m_stream << "error: " << error_msg << maybeNewline(error_msg);
+
+      // Print the value object directly.
+      m_options.DisableObjectiveC();
+    } else {
+      object_desc = *object_desc_or_err;
+    }
+  }
+
   if (ShouldPrintValueObject()) {
     PrintLocationIfNeeded();
     m_stream->Indent();
@@ -91,8 +124,10 @@ llvm::Error ValueObjectPrinter::PrintValueObject() {
   m_val_summary_ok =
       PrintValueAndSummaryIfNeeded(value_printed, summary_printed);
 
-  if (m_val_summary_ok)
+  if (m_val_summary_ok) {
+    PrintObjectDescriptionIfNeeded(object_desc);
     return PrintChildrenIfNeeded(value_printed, summary_printed);
+  }
   m_stream->EOL();
 
   return llvm::Error::success();
@@ -145,29 +180,6 @@ void ValueObjectPrinter::SetupMostSpecializedValue() {
          "SetupMostSpecialized value must compute a valid ValueObject");
 }
 
-llvm::Expected<std::string> ValueObjectPrinter::GetDescriptionForDisplay() {
-  ValueObject &valobj = GetMostSpecializedValue();
-  llvm::Expected<std::string> maybe_str = valobj.GetObjectDescription();
-  if (maybe_str)
-    return maybe_str;
-
-  if (maybe_str.errorIsA<lldb_private::ExpressionError>())
-    // Propagate expression errors to expose diagnostics to the user.
-    // Without this early exit, the summary/value may be shown without errors.
-    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;
-}
-
 const char *ValueObjectPrinter::GetRootNameForDisplay() {
   const char *root_valobj_name =
       m_options.m_root_valobj_name.empty()
@@ -474,38 +486,14 @@ bool 
ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed,
   return !error_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 << ' ';
-      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.
-        if (object_desc->empty() || object_desc->back() != '\n')
-          *m_stream << '\n';
-      }
-      return llvm::Error::success();
-    }
-  }
-  return llvm::Error::success();
+void ValueObjectPrinter::PrintObjectDescriptionIfNeeded(
+    std::optional<std::string> object_desc) {
+  if (!object_desc)
+    return;
+
+  if (!m_options.m_hide_value || ShouldShowName())
+    *m_stream << ' ';
+  *m_stream << *object_desc << maybeNewline(*object_desc);
 }
 
 bool DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const {
@@ -825,9 +813,6 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
 
 llvm::Error ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
                                                       bool summary_printed) {
-  auto error = PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
-  if (error)
-    return error;
 
   ValueObject &valobj = GetMostSpecializedValue();
 

>From aeb1e40e5747ebdc5b56df582f003e2d74b1b32b Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Thu, 14 Aug 2025 15:30:38 -0700
Subject: [PATCH 3/6] reset correct name option

---
 lldb/source/DataFormatters/DumpValueObjectOptions.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp 
b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
index 8ce53020e8195..fff0df8287126 100644
--- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
+++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
@@ -69,7 +69,7 @@ DumpValueObjectOptions 
&DumpValueObjectOptions::DisableObjectiveC() {
   // Reset these options to their default values.
   SetUseObjectiveC(false);
   SetHideRootType(false);
-  SetHideRootName(false);
+  SetHideName(false);
   SetHideValue(false);
   SetShowSummary(true);
   return *this;

>From 7db991430e13a5400208eb7bdedbc3fb59126168 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Thu, 14 Aug 2025 15:40:33 -0700
Subject: [PATCH 4/6] remove leftover #include

---
 lldb/source/DataFormatters/ValueObjectPrinter.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 4dde8bcde3a4c..1207cf7d2cb6b 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -9,7 +9,6 @@
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
 
 #include "lldb/DataFormatters/DataVisualization.h"
-#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"

>From f54992aa824a9651e5377ee5e4f39b95d33bda23 Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Thu, 14 Aug 2025 16:25:00 -0700
Subject: [PATCH 5/6] add test for failing description

---
 .../lang/objc/failing-description/Makefile    |  3 +++
 .../TestObjCFailingDescription.py             | 17 +++++++++++++++
 .../API/lang/objc/failing-description/main.m  | 21 +++++++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 lldb/test/API/lang/objc/failing-description/Makefile
 create mode 100644 
lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py
 create mode 100644 lldb/test/API/lang/objc/failing-description/main.m

diff --git a/lldb/test/API/lang/objc/failing-description/Makefile 
b/lldb/test/API/lang/objc/failing-description/Makefile
new file mode 100644
index 0000000000000..c8d46cd4cd21a
--- /dev/null
+++ b/lldb/test/API/lang/objc/failing-description/Makefile
@@ -0,0 +1,3 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+include Makefile.rules
diff --git 
a/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py 
b/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py
new file mode 100644
index 0000000000000..a06ff577e7f7d
--- /dev/null
+++ b/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py
@@ -0,0 +1,17 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "break here", 
lldb.SBFileSpec("main.m"))
+        self.expect(
+            "expr -O -- bad", substrs=["error:", "expression interrupted", 
"(Bad *) 0x"]
+        )
+        self.expect(
+            "dwim-print -O -- bad",
+            substrs=["error:", "expression interrupted", "_lookHere = NO"],
+        )
diff --git a/lldb/test/API/lang/objc/failing-description/main.m 
b/lldb/test/API/lang/objc/failing-description/main.m
new file mode 100644
index 0000000000000..7416e0a76544d
--- /dev/null
+++ b/lldb/test/API/lang/objc/failing-description/main.m
@@ -0,0 +1,21 @@
+#import <Foundation/Foundation.h>
+
+@interface Bad : NSObject
+@end
+
+@implementation Bad {
+  BOOL _lookHere;
+}
+
+- (NSString *)description {
+  int *i = NULL;
+  *i = 0;
+  return @"surprise";
+}
+@end
+
+int main() {
+  Bad *bad = [Bad new];
+  printf("break here\n");
+  return 0;
+}

>From 532bfeac138117d0a1d3c1e23ab9b20e9ffb768a Mon Sep 17 00:00:00 2001
From: Dave Lee <davelee....@gmail.com>
Date: Thu, 14 Aug 2025 16:35:22 -0700
Subject: [PATCH 6/6] add test for struct description

---
 .../API/lang/objc/struct-description/Makefile  |  2 ++
 .../TestObjCStructDescription.py               | 18 ++++++++++++++++++
 .../API/lang/objc/struct-description/main.m    | 12 ++++++++++++
 3 files changed, 32 insertions(+)
 create mode 100644 lldb/test/API/lang/objc/struct-description/Makefile
 create mode 100644 
lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py
 create mode 100644 lldb/test/API/lang/objc/struct-description/main.m

diff --git a/lldb/test/API/lang/objc/struct-description/Makefile 
b/lldb/test/API/lang/objc/struct-description/Makefile
new file mode 100644
index 0000000000000..d0aadc1af9e58
--- /dev/null
+++ b/lldb/test/API/lang/objc/struct-description/Makefile
@@ -0,0 +1,2 @@
+OBJC_SOURCES := main.m
+include Makefile.rules
diff --git 
a/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py 
b/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py
new file mode 100644
index 0000000000000..d152b2690c663
--- /dev/null
+++ b/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py
@@ -0,0 +1,18 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "break here", 
lldb.SBFileSpec("main.m"))
+        self.expect(
+            "vo pair",
+            substrs=["error:", "not a pointer type", "(Pair) pair = (f = 2, e 
= 3)"],
+        )
+        self.expect(
+            "expr -O -- pair",
+            substrs=["error:", "not a pointer type", "(Pair)  (f = 2, e = 3)"],
+        )
diff --git a/lldb/test/API/lang/objc/struct-description/main.m 
b/lldb/test/API/lang/objc/struct-description/main.m
new file mode 100644
index 0000000000000..5031a28437e60
--- /dev/null
+++ b/lldb/test/API/lang/objc/struct-description/main.m
@@ -0,0 +1,12 @@
+#include <stdio.h>
+
+typedef struct Pair {
+  int f;
+  int e;
+} Pair;
+
+int main() {
+  Pair pair = {2, 3};
+  printf("break here\n");
+  return 0;
+}

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to