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/2] [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/2] 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(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits