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 <apra...@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 ®_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,@object # @e .section .rodata,"a",@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 21fab8da49b1f97b5cc107fb6adac8aaba9e196e Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@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 | 18 +++-- .../GNUstepObjCRuntime/GNUstepObjCRuntime.h | 6 +- 12 files changed, 143 insertions(+), 103 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..58b838752be50 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp @@ -104,15 +104,17 @@ 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( - Stream &strm, Value &value, ExecutionContextScope *exe_scope) { - // TODO: ObjC has a generic way to do this - return false; + +llvm::Error +GNUstepObjCRuntime::GetObjectDescription(Stream &strm, Value &value, + ExecutionContextScope *exe_scope) { + 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..de24466ebb003 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h @@ -57,10 +57,10 @@ 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 78a7a7e28afef7d34cb91ae155f112eb03c3d94d Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@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 971c68be32d76e5450c9d61f336318d362972b6c Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@apple.com> Date: Tue, 18 Jun 2024 13:44:45 -0700 Subject: [PATCH 4/4] Reformat test (NFC) --- .../API/commands/dwim-print/TestDWIMPrint.py | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py index 785a9621c99dc..18b23e2350ecd 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. @@ -162,7 +154,5 @@ def test_preserves_persistent_variables(self): 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") - ) + lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) self.expect("dwim-print -O -- opaque", substrs=['0x']) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits