diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h index cdb620e2148de..70166f33cfc45 100644 --- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h +++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h @@ -76,7 +76,9 @@ class DumpValueObjectOptions { DumpValueObjectOptions &SetShowLocation(bool show = false); - DumpValueObjectOptions &SetUseObjectiveC(bool use = false); + DumpValueObjectOptions &DisableObjectDescription(); + + DumpValueObjectOptions &SetUseObjectDescription(bool use = false); DumpValueObjectOptions &SetShowSummary(bool show = true); @@ -143,13 +145,14 @@ class DumpValueObjectOptions { ChildPrintingDecider m_child_printing_decider; PointerAsArraySettings m_pointer_as_array; unsigned m_expand_ptr_type_flags = 0; + // The following flags commonly default to false. 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_use_object_desc : 1; bool m_hide_root_type : 1; bool m_hide_root_name : 1; bool m_hide_name : 1; 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 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 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/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h b/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h index ebf26cea95cf3..4fd552a6700b8 100644 --- a/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h +++ b/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h @@ -31,7 +31,7 @@ class OptionGroupValueObjectDisplay : public OptionGroup { bool AnyOptionWasSet() const { return show_types || no_summary_depth != 0 || show_location || - flat_output || use_objc || max_depth != UINT32_MAX || + flat_output || use_object_desc || max_depth != UINT32_MAX || ptr_depth != 0 || !use_synth || be_raw || ignore_cap || run_validator; } @@ -42,7 +42,7 @@ class OptionGroupValueObjectDisplay : public OptionGroup { lldb::Format format = lldb::eFormatDefault, lldb::TypeSummaryImplSP summary_sp = lldb::TypeSummaryImplSP()); - bool show_types : 1, show_location : 1, flat_output : 1, use_objc : 1, + bool show_types : 1, show_location : 1, flat_output : 1, use_object_desc : 1, use_synth : 1, be_raw : 1, ignore_cap : 1, run_validator : 1, max_depth_is_default : 1; diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp index 763c7b02e8c17..9e771f7a45d87 100644 --- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp +++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp @@ -92,7 +92,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, dump_options.SetHideRootName(suppress_result) .SetExpandPointerTypeFlags(lldb::eTypeIsObjC); - bool is_po = m_varobj_options.use_objc; + bool is_po = m_varobj_options.use_object_desc; StackFrame *frame = m_exe_ctx.GetFramePtr(); diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index d2eaa4acbc568..21c56b6db794d 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -220,7 +220,7 @@ EvaluateExpressionOptions CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions( const Target &target, const OptionGroupValueObjectDisplay &display_opts) { EvaluateExpressionOptions options; - options.SetCoerceToId(display_opts.use_objc); + options.SetCoerceToId(display_opts.use_object_desc); options.SetUnwindOnError(unwind_on_error); options.SetIgnoreBreakpoints(ignore_breakpoints); options.SetKeepInMemory(true); @@ -263,11 +263,11 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions( bool CommandObjectExpression::CommandOptions::ShouldSuppressResult( const OptionGroupValueObjectDisplay &display_opts) const { // Explicitly disabling persistent results takes precedence over the - // m_verbosity/use_objc logic. + // m_verbosity/use_object_desc logic. if (suppress_persistent_result != eLazyBoolCalculate) return suppress_persistent_result == eLazyBoolYes; - return display_opts.use_objc && + return display_opts.use_object_desc && m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact; } @@ -356,7 +356,7 @@ Options *CommandObjectExpression::GetOptions() { return &m_option_group; } void CommandObjectExpression::HandleCompletion(CompletionRequest &request) { EvaluateExpressionOptions options; - options.SetCoerceToId(m_varobj_options.use_objc); + options.SetCoerceToId(m_varobj_options.use_object_desc); options.SetLanguage(m_command_options.language); options.SetExecutionPolicy(lldb_private::eExecutionPolicyNever); options.SetAutoApplyFixIts(false); diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp index c343e6083df19..e1df9522256fa 100644 --- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp +++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp @@ -17,7 +17,7 @@ DumpValueObjectOptions::DumpValueObjectOptions() : m_summary_sp(), m_root_valobj_name(), m_decl_printing_helper(), m_child_printing_decider(), 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_show_types(false), m_show_location(false), m_use_object_desc(false), 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), @@ -65,8 +65,19 @@ DumpValueObjectOptions &DumpValueObjectOptions::SetShowLocation(bool show) { return *this; } -DumpValueObjectOptions &DumpValueObjectOptions::SetUseObjectiveC(bool use) { - m_use_objc = use; +DumpValueObjectOptions &DumpValueObjectOptions::DisableObjectDescription() { + // Reset these options to their default values. + SetUseObjectDescription(false); + SetHideRootType(false); + SetHideName(false); + SetHideValue(false); + SetShowSummary(true); + return *this; +} + +DumpValueObjectOptions & +DumpValueObjectOptions::SetUseObjectDescription(bool use) { + m_use_object_desc = use; return *this; } diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp index 40493df8aec37..676598686de75 100644 --- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp +++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp @@ -14,8 +14,10 @@ #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 +#include using namespace lldb; using namespace lldb_private; @@ -69,6 +71,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_object_desc && !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 @@ -77,6 +91,25 @@ llvm::Error ValueObjectPrinter::PrintValueObject() { !m_orig_valobj.GetCompilerType().IsValid()) return m_orig_valobj.GetError().ToError(); + std::optional 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 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.DisableObjectDescription(); + } else { + object_desc = *object_desc_or_err; + } + } + if (ShouldPrintValueObject()) { PrintLocationIfNeeded(); m_stream->Indent(); @@ -90,8 +123,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(); @@ -144,24 +179,6 @@ void ValueObjectPrinter::SetupMostSpecializedValue() { "SetupMostSpecialized value must compute a valid ValueObject"); } -llvm::Expected ValueObjectPrinter::GetDescriptionForDisplay() { - ValueObject &valobj = GetMostSpecializedValue(); - llvm::Expected 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; -} - const char *ValueObjectPrinter::GetRootNameForDisplay() { const char *root_valobj_name = m_options.m_root_valobj_name.empty() @@ -468,38 +485,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 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 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 { @@ -524,7 +517,7 @@ bool ValueObjectPrinter::ShouldPrintChildren( if (m_options.m_pointer_as_array) return true; - if (m_options.m_use_objc) + if (m_options.m_use_object_desc) return false; bool print_children = true; @@ -819,9 +812,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(); diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp index 060822d8f14db..8d918f415ac16 100644 --- a/lldb/source/Expression/REPL.cpp +++ b/lldb/source/Expression/REPL.cpp @@ -321,7 +321,7 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { const bool colorize_err = error_sp->GetFile().GetIsTerminalWithColors(); EvaluateExpressionOptions expr_options = m_expr_options; - expr_options.SetCoerceToId(m_varobj_options.use_objc); + expr_options.SetCoerceToId(m_varobj_options.use_object_desc); expr_options.SetKeepInMemory(true); expr_options.SetUseDynamic(m_varobj_options.use_dynamic); expr_options.SetGenerateDebugInfo(true); diff --git a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp index cbef9bb9c4d04..c5d547f74e108 100644 --- a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp +++ b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp @@ -91,7 +91,7 @@ Status OptionGroupValueObjectDisplay::SetOptionValue( flat_output = true; break; case 'O': - use_objc = true; + use_object_desc = true; break; case 'R': be_raw = true; @@ -164,7 +164,7 @@ void OptionGroupValueObjectDisplay::OptionParsingStarting( no_summary_depth = 0; show_location = false; flat_output = false; - use_objc = false; + use_object_desc = false; max_depth = UINT32_MAX; max_depth_is_default = true; ptr_depth = 0; @@ -192,14 +192,14 @@ DumpValueObjectOptions OptionGroupValueObjectDisplay::GetAsDumpOptions( lldb::Format format, lldb::TypeSummaryImplSP summary_sp) { DumpValueObjectOptions options; options.SetMaximumPointerDepth(ptr_depth); - if (use_objc) + if (use_object_desc) options.SetShowSummary(false); else options.SetOmitSummaryDepth(no_summary_depth); options.SetMaximumDepth(max_depth, max_depth_is_default) .SetShowTypes(show_types) .SetShowLocation(show_location) - .SetUseObjectiveC(use_objc) + .SetUseObjectDescription(use_object_desc) .SetUseDynamicType(use_dynamic) .SetUseSyntheticValue(use_synth) .SetFlatOutput(flat_output) @@ -209,8 +209,9 @@ DumpValueObjectOptions OptionGroupValueObjectDisplay::GetAsDumpOptions( if (lang_descr_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact) - options.SetHideRootType(use_objc).SetHideName(use_objc).SetHideValue( - use_objc); + options.SetHideRootType(use_object_desc) + .SetHideName(use_object_desc) + .SetHideValue(use_object_desc); if (be_raw) options.SetRawDisplay(); 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 + +@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; +} 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 + +typedef struct Pair { + int f; + int e; +} Pair; + +int main() { + Pair pair = {2, 3}; + printf("break here\n"); + return 0; +} diff --git a/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py b/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py index bb069a9ad3215..ca543525dc038 100644 --- a/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py +++ b/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py @@ -81,9 +81,11 @@ def check(value): check(value) - self.expect('dwim-print -O -- strct', error=True, - substrs=['Missing type']) - + self.expect( + "dwim-print -O -- strct", + substrs=["error: Missing type", "properties = true"], + ) + process.Continue() self.expect('expression -O -- number', error=True, substrs=['self', 'not', 'found']) diff --git a/lldb/test/API/lang/swift/expression/error_reporting/main.swift b/lldb/test/API/lang/swift/expression/error_reporting/main.swift index 2ffd608be0b06..ba8f02ce07a03 100644 --- a/lldb/test/API/lang/swift/expression/error_reporting/main.swift +++ b/lldb/test/API/lang/swift/expression/error_reporting/main.swift @@ -7,7 +7,7 @@ class State { var number : Int } -struct S {} +struct S { var properties: Bool = true } func f(_ strct : S) { print("in function") // break here