https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/106470
>From 4f57669afd3d9e7475c0e44976eaa9b534883152 Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@apple.com> Date: Wed, 28 Aug 2024 16:19:21 -0700 Subject: [PATCH] [lldb] Inline expression evaluator error visualization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch is a reworking of Pete Lawrence's (@PortalPete) proposal for better expression evaluator error messages: https://github.com/llvm/llvm-project/pull/80938 Before: ``` $ lldb -o "expr a+b" (lldb) expr a+b error: <user expression 0>:1:1: use of undeclared identifier 'a' a+b ^ error: <user expression 0>:1:3: use of undeclared identifier 'b' a+b ^ ``` After: ``` (lldb) expr a+b ^ ^ │ ╰─ error: use of undeclared identifier 'b' ╰─ error: use of undeclared identifier 'a' ``` This eliminates the confusing `<user expression 0>:1:3` source location and avoids echoing the expression to the console again, which results in a cleaner presentation that makes it easier to grasp what's going on. --- lldb/include/lldb/API/SBDebugger.h | 2 + lldb/include/lldb/Core/Debugger.h | 4 + .../lldb/Expression/DiagnosticManager.h | 1 + lldb/include/lldb/Interpreter/CommandObject.h | 8 + lldb/source/API/SBDebugger.cpp | 6 + .../Commands/CommandObjectExpression.cpp | 154 ++++++++++++++++-- lldb/source/Core/CoreProperties.td | 4 + lldb/source/Core/Debugger.cpp | 13 +- lldb/source/Expression/DiagnosticManager.cpp | 2 +- .../source/Interpreter/CommandInterpreter.cpp | 4 +- .../Clang/ClangExpressionParser.cpp | 27 +-- .../ctf/CommandObjectThreadTraceExportCTF.h | 2 +- .../diagnostics/TestExprDiagnostics.py | 51 ++++++ .../TestPersistentVariables.py | 4 +- .../TestStaticInitializers.py | 2 +- .../TestTemplateFunctions.py | 3 +- .../test/API/lang/mixed/TestMixedLanguages.py | 2 +- lldb/test/Shell/Expr/TestObjCIDCast.test | 2 +- .../test/Shell/Expr/TestObjCInCXXContext.test | 2 +- .../NativePDB/incomplete-tag-type.cpp | 4 +- lldb/tools/driver/Driver.cpp | 1 + .../Expression/DiagnosticManagerTest.cpp | 33 ++-- lldb/unittests/Interpreter/CMakeLists.txt | 1 + .../TestCommandObjectExpression.cpp | 34 ++++ 24 files changed, 315 insertions(+), 51 deletions(-) create mode 100644 lldb/unittests/Interpreter/TestCommandObjectExpression.cpp diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index 84ea9c0f772e16..6afa1c932ab050 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -304,6 +304,8 @@ class LLDB_API SBDebugger { bool GetUseColor() const; + bool SetShowInlineDiagnostics(bool); + bool SetUseSourceCache(bool use_source_cache); bool GetUseSourceCache() const; diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index a72c2596cc2c5e..1d5f2fcc20626c 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -364,6 +364,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>, const std::string &GetInstanceName() { return m_instance_name; } + bool GetShowInlineDiagnostics() const; + + bool SetShowInlineDiagnostics(bool); + bool LoadPlugin(const FileSpec &spec, Status &error); void RunIOHandlers(); diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 62c6bcefe54110..b9a6421577781e 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -39,6 +39,7 @@ struct DiagnosticDetail { unsigned line = 0; uint16_t column = 0; uint16_t length = 0; + bool hidden = false; bool in_user_input = false; }; /// Contains {{}, 1, 3, 3, true} in the example above. diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index 20c4769af90338..c5167e5e0ecb6a 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -340,6 +340,13 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> { return false; } + /// Set the command input as it appeared in the terminal. This + /// is used to have errors refer directly to the original command. + void SetOriginalCommandString(std::string s) { m_original_command = s; } + + /// \param offset_in_command is on what column \c args_string + /// appears, if applicable. This enables diagnostics that refer back + /// to the user input. virtual void Execute(const char *args_string, CommandReturnObject &result) = 0; @@ -404,6 +411,7 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> { std::string m_cmd_help_short; std::string m_cmd_help_long; std::string m_cmd_syntax; + std::string m_original_command; Flags m_flags; std::vector<CommandArgumentEntry> m_arguments; lldb::CommandOverrideCallback m_deprecated_command_override_callback; diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 6b72994fc96afb..47931f1c16f9a3 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -1483,6 +1483,12 @@ bool SBDebugger::GetUseColor() const { return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false); } +bool SBDebugger::SetShowInlineDiagnostics(bool value) { + LLDB_INSTRUMENT_VA(this, value); + + return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false); +} + bool SBDebugger::SetUseSourceCache(bool value) { LLDB_INSTRUMENT_VA(this, value); diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index 771194638e1b65..49f4421fdb54f9 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -10,6 +10,7 @@ #include "CommandObjectExpression.h" #include "lldb/Core/Debugger.h" +#include "lldb/Expression/DiagnosticManager.h" #include "lldb/Expression/ExpressionVariable.h" #include "lldb/Expression/REPL.h" #include "lldb/Expression/UserExpression.h" @@ -398,6 +399,122 @@ CanBeUsedForElementCountPrinting(ValueObject &valobj) { return Status(); } +static llvm::raw_ostream &PrintSeverity(Stream &stream, + lldb::Severity severity) { + llvm::HighlightColor color; + llvm::StringRef text; + switch (severity) { + case eSeverityError: + color = llvm::HighlightColor::Error; + text = "error: "; + break; + case eSeverityWarning: + color = llvm::HighlightColor::Warning; + text = "warning: "; + break; + case eSeverityInfo: + color = llvm::HighlightColor::Remark; + text = "note: "; + break; + } + return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable) + << text; +} + +namespace lldb_private { +// Public for unittesting. +void RenderDiagnosticDetails(Stream &stream, + std::optional<uint16_t> offset_in_command, + bool show_inline, + llvm::ArrayRef<DiagnosticDetail> details) { + if (details.empty()) + return; + + if (!offset_in_command) { + for (const DiagnosticDetail &detail : details) { + PrintSeverity(stream, detail.severity); + stream << detail.rendered << '\n'; + } + return; + } + + // Print a line with caret indicator(s) below the lldb prompt + command. + const size_t padding = *offset_in_command; + stream << std::string(padding, ' '); + + size_t offset = 1; + std::vector<DiagnosticDetail> remaining_details, other_details, + hidden_details; + for (const DiagnosticDetail &detail : details) { + if (!show_inline || !detail.source_location) { + other_details.push_back(detail); + continue; + } + if (detail.source_location->hidden) { + hidden_details.push_back(detail); + continue; + } + if (!detail.source_location->in_user_input) { + other_details.push_back(detail); + continue; + } + + auto &loc = *detail.source_location; + remaining_details.push_back(detail); + if (offset > loc.column) + continue; + stream << std::string(loc.column - offset, ' ') << '^'; + if (loc.length > 1) + stream << std::string(loc.length - 1, '~'); + offset = loc.column + 1; + } + stream << '\n'; + + // Work through each detail in reverse order using the vector/stack. + bool did_print = false; + for (auto detail = remaining_details.rbegin(); + detail != remaining_details.rend(); + ++detail, remaining_details.pop_back()) { + // Get the information to print this detail and remove it from the stack. + // Print all the lines for all the other messages first. + stream << std::string(padding, ' '); + size_t offset = 1; + for (auto &remaining_detail : + llvm::ArrayRef(remaining_details).drop_back(1)) { + uint16_t column = remaining_detail.source_location->column; + stream << std::string(column - offset, ' ') << "│"; + offset = column + 1; + } + + // Print the line connecting the ^ with the error message. + uint16_t column = detail->source_location->column; + if (offset <= column) + stream << std::string(column - offset, ' ') << "╰─ "; + + // Print a colorized string based on the message's severity type. + PrintSeverity(stream, detail->severity); + + // Finally, print the message and start a new line. + stream << detail->message << '\n'; + did_print = true; + } + + // Print the non-located details. + for (const DiagnosticDetail &detail : other_details) { + PrintSeverity(stream, detail.severity); + stream << detail.rendered << '\n'; + did_print = true; + } + + // Print the hidden details as a last resort. + if (!did_print) + for (const DiagnosticDetail &detail : hidden_details) { + PrintSeverity(stream, detail.severity); + stream << detail.rendered << '\n'; + } +} +} // namespace lldb_private + bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr, Stream &output_stream, Stream &error_stream, @@ -486,19 +603,34 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr, result.SetStatus(eReturnStatusSuccessFinishResult); } else { - const char *error_cstr = result_valobj_sp->GetError().AsCString(); - if (error_cstr && error_cstr[0]) { - const size_t error_cstr_len = strlen(error_cstr); - const bool ends_with_newline = error_cstr[error_cstr_len - 1] == '\n'; - if (strstr(error_cstr, "error:") != error_cstr) - error_stream.PutCString("error: "); - error_stream.Write(error_cstr, error_cstr_len); - if (!ends_with_newline) - error_stream.EOL(); + // Retrieve the diagnostics. + std::vector<DiagnosticDetail> details; + llvm::consumeError(llvm::handleErrors( + result_valobj_sp->GetError().ToError(), + [&](ExpressionError &error) { details = error.GetDetails(); })); + // Find the position of the expression in the command. + std::optional<uint16_t> expr_pos; + size_t nchar = m_original_command.find(expr); + if (nchar != std::string::npos) + expr_pos = nchar + GetDebugger().GetPrompt().size(); + + if (!details.empty()) { + bool show_inline = + GetDebugger().GetShowInlineDiagnostics() && !expr.contains('\n'); + RenderDiagnosticDetails(error_stream, expr_pos, show_inline, details); } else { - error_stream.PutCString("error: unknown error\n"); + const char *error_cstr = result_valobj_sp->GetError().AsCString(); + llvm::StringRef error(error_cstr); + if (!error.empty()) { + if (!error.starts_with("error:")) + error_stream << "error: "; + error_stream << error; + if (!error.ends_with('\n')) + error_stream.EOL(); + } else { + error_stream << "error: unknown error\n"; + } } - result.SetStatus(eReturnStatusFailed); } } diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td index a6cb951187a040..e11aad2660b461 100644 --- a/lldb/source/Core/CoreProperties.td +++ b/lldb/source/Core/CoreProperties.td @@ -225,4 +225,8 @@ let Definition = "debugger" in { DefaultEnumValue<"eDWIMPrintVerbosityNone">, EnumValues<"OptionEnumValues(g_dwim_print_verbosities)">, Desc<"The verbosity level used by dwim-print.">; + def ShowInlineDiagnostics: Property<"show-inline-diagnostics", "Boolean">, + Global, + DefaultFalse, + Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">; } diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 9bdc5a3949751d..e6b9eedd89b4e3 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -592,7 +592,18 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const { const uint32_t idx = ePropertyDWIMPrintVerbosity; return GetPropertyAtIndexAs<lldb::DWIMPrintVerbosity>( idx, static_cast<lldb::DWIMPrintVerbosity>( - g_debugger_properties[idx].default_uint_value)); + g_debugger_properties[idx].default_uint_value != 0)); +} + +bool Debugger::GetShowInlineDiagnostics() const { + const uint32_t idx = ePropertyShowInlineDiagnostics; + return GetPropertyAtIndexAs<bool>( + idx, g_debugger_properties[idx].default_uint_value); +} + +bool Debugger::SetShowInlineDiagnostics(bool b) { + const uint32_t idx = ePropertyShowInlineDiagnostics; + return SetPropertyAtIndex(idx, b); } #pragma mark Debugger diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp index 3731a200c5e2f7..7c67a0ce4aa026 100644 --- a/lldb/source/Expression/DiagnosticManager.cpp +++ b/lldb/source/Expression/DiagnosticManager.cpp @@ -37,7 +37,7 @@ ExpressionError::ExpressionError(lldb::ExpressionResults result, : ErrorInfo(std::error_code(result, expression_category())), m_message(msg), m_details(details) {} -static const char *StringForSeverity(lldb::Severity severity) { +static llvm::StringRef StringForSeverity(lldb::Severity severity) { switch (severity) { // this should be exhaustive case lldb::eSeverityError: diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index acd592c3bd2dbc..d17aa6fec1f00e 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1887,7 +1887,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line, CommandReturnObject &result, bool force_repeat_command) { std::string command_string(command_line); - std::string original_command_string(command_line); + std::string original_command_string(command_string); + std::string real_original_command_string(command_string); Log *log = GetLog(LLDBLog::Commands); llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")", @@ -2076,6 +2077,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line, } ElapsedTime elapsed(execute_time); + cmd_obj->SetOriginalCommandString(real_original_command_string); cmd_obj->Execute(remainder.c_str(), result); } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 687962c700a30b..9b056ea73a77fb 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -176,15 +176,22 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { m_manager = manager; } - /// Returns the last ClangDiagnostic message that the DiagnosticManager - /// received or a nullptr if the DiagnosticMangager hasn't seen any - /// Clang diagnostics yet. + /// Returns the last error ClangDiagnostic message that the + /// DiagnosticManager received or a nullptr. ClangDiagnostic *MaybeGetLastClangDiag() const { if (m_manager->Diagnostics().empty()) return nullptr; - lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get(); - ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag); - return clang_diag; + auto &diags = m_manager->Diagnostics(); + for (auto it = diags.rbegin(); it != diags.rend(); it++) { + lldb_private::Diagnostic *diag = it->get(); + if (ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag)) { + if (clang_diag->GetSeverity() == lldb::eSeverityWarning) + return nullptr; + if (clang_diag->GetSeverity() == lldb::eSeverityError) + return clang_diag; + } + } + return nullptr; } void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, @@ -214,8 +221,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { m_passthrough->HandleDiagnostic(DiagLevel, Info); DiagnosticDetail detail; - bool make_new_diagnostic = true; - switch (DiagLevel) { case DiagnosticsEngine::Level::Fatal: case DiagnosticsEngine::Level::Error: @@ -229,9 +234,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { detail.severity = lldb::eSeverityInfo; break; case DiagnosticsEngine::Level::Note: - m_manager->AppendMessageToDiagnostic(m_output); - make_new_diagnostic = false; - // 'note:' diagnostics for errors and warnings can also contain Fix-Its. // We add these Fix-Its to the last error diagnostic to make sure // that we later have all Fix-Its related to an 'error' diagnostic when @@ -249,7 +251,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { AddAllFixIts(clang_diag, Info); break; } - if (make_new_diagnostic) { // ClangDiagnostic messages are expected to have no whitespace/newlines // around them. std::string stripped_output = @@ -269,6 +270,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { loc.line = fsloc.getSpellingLineNumber(); loc.column = fsloc.getSpellingColumnNumber(); loc.in_user_input = filename == m_filename; + loc.hidden = filename.starts_with("<lldb wrapper "); // Find the range of the primary location. for (const auto &range : Info.getRanges()) { @@ -298,7 +300,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { AddAllFixIts(new_diagnostic.get(), Info); m_manager->AddDiagnostic(std::move(new_diagnostic)); - } } void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override { diff --git a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h index 1a034e87cfb65b..06834edf14ea16 100644 --- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h +++ b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h @@ -48,7 +48,7 @@ class CommandObjectThreadTraceExportCTF : public CommandObjectParsed { Options *GetOptions() override { return &m_options; } protected: - void DoExecute(Args &command, CommandReturnObject &result) override; + void DoExecute(Args &args, CommandReturnObject &result) override; CommandOptions m_options; }; diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py index ddc1c3598480cf..1687b617350d92 100644 --- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py +++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py @@ -183,3 +183,54 @@ def test_source_locations_from_objc_modules(self): # The NSLog definition source line should be printed. Return value and # the first argument are probably stable enough that this test can check for them. self.assertIn("void NSLog(NSString *format", value.GetError().GetCString()) + + def test_command_expr_formatting(self): + """Test that the source and caret positions LLDB prints are correct""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "// Break here", self.main_source_spec + ) + frame = thread.GetFrameAtIndex(0) + self.expect("settings set show-inline-diagnostics true") + + def check(input_ref): + self.expect(input_ref[0], error=True, substrs=input_ref[1:]) + + check( + [ + "expression -- a+b", + " ^ ^", + " │ ╰─ error: use of undeclared identifier 'b'", + " ╰─ error: use of undeclared identifier 'a'", + ] + ) + + check( + [ + "expr -- a", + " ^", + " ╰─ error: use of undeclared identifier 'a'", + ] + ) + check( + [ + "expr -i 0 -o 0 -- a", + " ^", + " ╰─ error: use of undeclared identifier 'a'", + ] + ) + + self.expect( + "expression --top-level -- template<typename T> T FOO(T x) { return x/2;}" + ) + check( + [ + 'expression -- FOO("")', + " ^", + " ╰─ note: in instantiation of function template specialization 'FOO<const char *>' requested here", + "error: <user expression", + "invalid operands to binary expression", + ] + ) + check(["expression --\na\n+\nb", "error: <user", "a", "error: <user", "b"]) diff --git a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py index 6a7995ff2a837e..d0d9358589a719 100644 --- a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py +++ b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py @@ -56,7 +56,7 @@ def test_persistent_variables(self): self.expect( "expr int $i = 123", error=True, - substrs=["error: redefinition of persistent variable '$i'"], + substrs=["redefinition of persistent variable '$i'"], ) self.expect_expr("$i", result_type="int", result_value="5") @@ -65,7 +65,7 @@ def test_persistent_variables(self): self.expect( "expr long $i = 123", error=True, - substrs=["error: redefinition of persistent variable '$i'"], + substrs=["redefinition of persistent variable '$i'"], ) self.expect_expr("$i", result_type="int", result_value="5") diff --git a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py index ea3aa6a4608c41..c734033bd6c028 100644 --- a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py +++ b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py @@ -35,7 +35,7 @@ def test_failing_init(self): self.expect( "expr -p -- struct Foo2 { Foo2() { do_abort(); } }; Foo2 f;", error=True, - substrs=["error: couldn't run static initializer:"], + substrs=["couldn't run static initializer:"], ) def test_without_process(self): diff --git a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py index 71df90e6a6d161..3be93dedfd11df 100644 --- a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py +++ b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py @@ -21,7 +21,8 @@ def do_test_template_function(self, add_cast): "expr b1 <=> b2", error=True, substrs=[ - "warning: <user expression 0>:1:4: '<=>' is a single token in C++20; add a space to avoid a change in behavior" + "warning:", + "'<=>' is a single token in C++20; add a space to avoid a change in behavior", ], ) diff --git a/lldb/test/API/lang/mixed/TestMixedLanguages.py b/lldb/test/API/lang/mixed/TestMixedLanguages.py index 8b73254cce4a93..1637d59a5edcba 100644 --- a/lldb/test/API/lang/mixed/TestMixedLanguages.py +++ b/lldb/test/API/lang/mixed/TestMixedLanguages.py @@ -40,7 +40,7 @@ def cleanup(): self.runCmd("run") self.expect("thread backtrace", substrs=["`main", "lang=c"]) # Make sure evaluation of C++11 fails. - self.expect("expr foo != nullptr", error=True, startstr="error") + self.expect("expr foo != nullptr", error=True, substrs=["error"]) # Run to BP at foo (in foo.cpp) and test that the language is C++. self.runCmd("breakpoint set -n foo") diff --git a/lldb/test/Shell/Expr/TestObjCIDCast.test b/lldb/test/Shell/Expr/TestObjCIDCast.test index 0611171da09e2e..19ca404643c1d9 100644 --- a/lldb/test/Shell/Expr/TestObjCIDCast.test +++ b/lldb/test/Shell/Expr/TestObjCIDCast.test @@ -6,4 +6,4 @@ // RUN: 2>&1 | FileCheck %s // CHECK: (lldb) expression --language objc -- *(id)0x1 -// CHECK: error: Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory +// CHECK: error:{{.*}}Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory diff --git a/lldb/test/Shell/Expr/TestObjCInCXXContext.test b/lldb/test/Shell/Expr/TestObjCInCXXContext.test index 8537799bdeb674..f8cad5b58a1e53 100644 --- a/lldb/test/Shell/Expr/TestObjCInCXXContext.test +++ b/lldb/test/Shell/Expr/TestObjCInCXXContext.test @@ -18,4 +18,4 @@ // CHECK-NEXT: (NSString *){{.*}}= nil // CHECK: (lldb) expression NSString -// CHECK-NEXT: error:{{.*}} use of undeclared identifier 'NSString' +// CHECK: error:{{.*}}use of undeclared identifier 'NSString' diff --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp index a249057282d890..8c168286903014 100644 --- a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp +++ b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp @@ -13,9 +13,7 @@ // CHECK: (lldb) expression d // CHECK: (D) $1 = {} // CHECK: (lldb) expression static_e_ref -// CHECK: error: {{.*}}incomplete type 'E' where a complete type is required -// CHECK: static_e_ref -// CHECK: ^ +// CHECK: error:{{.*}}incomplete type 'E' where a complete type is required // Complete base class. struct A { int x; A(); }; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index 14371da64f2f2f..afb1a1ff95c3a1 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -442,6 +442,7 @@ int Driver::MainLoop() { m_debugger.SetInputFileHandle(stdin, false); m_debugger.SetUseExternalEditor(m_option_data.m_use_external_editor); + m_debugger.SetShowInlineDiagnostics(true); struct winsize window_size; if ((isatty(STDIN_FILENO) != 0) && diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp index 4608b779f79a96..7e04d4b023e4c2 100644 --- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp +++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp @@ -72,18 +72,25 @@ TEST(DiagnosticManagerTest, HasFixits) { EXPECT_TRUE(mgr.HasFixIts()); } +static std::string toString(DiagnosticManager &mgr) { + // The error code doesn't really matter since we just convert the + // diagnostics to a string. + auto result = lldb::eExpressionCompleted; + return llvm::toString(mgr.GetAsError(result)); +} + TEST(DiagnosticManagerTest, GetStringNoDiags) { DiagnosticManager mgr; - EXPECT_EQ("", mgr.GetString()); + EXPECT_EQ("", toString(mgr)); std::unique_ptr<Diagnostic> empty; mgr.AddDiagnostic(std::move(empty)); - EXPECT_EQ("", mgr.GetString()); + EXPECT_EQ("", toString(mgr)); } TEST(DiagnosticManagerTest, GetStringBasic) { DiagnosticManager mgr; mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError)); - EXPECT_EQ("error: abc\n", mgr.GetString()); + EXPECT_EQ("error: abc\n", toString(mgr)); } TEST(DiagnosticManagerTest, GetStringMultiline) { @@ -91,15 +98,15 @@ TEST(DiagnosticManagerTest, GetStringMultiline) { // Multiline diagnostics should only get one severity label. mgr.AddDiagnostic(std::make_unique<TextDiag>("b\nc", lldb::eSeverityError)); - EXPECT_EQ("error: b\nc\n", mgr.GetString()); + EXPECT_EQ("error: b\nc\n", toString(mgr)); } TEST(DiagnosticManagerTest, GetStringMultipleDiags) { DiagnosticManager mgr; mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError)); - EXPECT_EQ("error: abc\n", mgr.GetString()); + EXPECT_EQ("error: abc\n", toString(mgr)); mgr.AddDiagnostic(std::make_unique<TextDiag>("def", lldb::eSeverityError)); - EXPECT_EQ("error: abc\nerror: def\n", mgr.GetString()); + EXPECT_EQ("error: abc\nerror: def\n", toString(mgr)); } TEST(DiagnosticManagerTest, GetStringSeverityLabels) { @@ -110,7 +117,7 @@ TEST(DiagnosticManagerTest, GetStringSeverityLabels) { mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning)); // Remarks have no labels. mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo)); - EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", mgr.GetString()); + EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", toString(mgr)); } TEST(DiagnosticManagerTest, GetStringPreserveOrder) { @@ -120,7 +127,7 @@ TEST(DiagnosticManagerTest, GetStringPreserveOrder) { mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo)); mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning)); mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError)); - EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", mgr.GetString()); + EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", toString(mgr)); } TEST(DiagnosticManagerTest, AppendMessageNoDiag) { @@ -139,7 +146,7 @@ TEST(DiagnosticManagerTest, AppendMessageAttachToLastDiag) { // This should append to 'bar' and not to 'foo'. mgr.AppendMessageToDiagnostic("message text"); - EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", mgr.GetString()); + EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", toString(mgr)); } TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) { @@ -150,7 +157,7 @@ TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) { // Pushing another diag after the message should work fine. mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError)); - EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", mgr.GetString()); + EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", toString(mgr)); } TEST(DiagnosticManagerTest, PutString) { @@ -159,7 +166,7 @@ TEST(DiagnosticManagerTest, PutString) { mgr.PutString(lldb::eSeverityError, "foo"); EXPECT_EQ(1U, mgr.Diagnostics().size()); EXPECT_EQ(eDiagnosticOriginLLDB, mgr.Diagnostics().front()->getKind()); - EXPECT_EQ("error: foo\n", mgr.GetString()); + EXPECT_EQ("error: foo\n", toString(mgr)); } TEST(DiagnosticManagerTest, PutStringMultiple) { @@ -169,7 +176,7 @@ TEST(DiagnosticManagerTest, PutStringMultiple) { mgr.PutString(lldb::eSeverityError, "foo"); mgr.PutString(lldb::eSeverityError, "bar"); EXPECT_EQ(2U, mgr.Diagnostics().size()); - EXPECT_EQ("error: foo\nerror: bar\n", mgr.GetString()); + EXPECT_EQ("error: foo\nerror: bar\n", toString(mgr)); } TEST(DiagnosticManagerTest, PutStringSeverities) { @@ -180,7 +187,7 @@ TEST(DiagnosticManagerTest, PutStringSeverities) { mgr.PutString(lldb::eSeverityError, "foo"); mgr.PutString(lldb::eSeverityWarning, "bar"); EXPECT_EQ(2U, mgr.Diagnostics().size()); - EXPECT_EQ("error: foo\nwarning: bar\n", mgr.GetString()); + EXPECT_EQ("error: foo\nwarning: bar\n", toString(mgr)); } TEST(DiagnosticManagerTest, FixedExpression) { diff --git a/lldb/unittests/Interpreter/CMakeLists.txt b/lldb/unittests/Interpreter/CMakeLists.txt index 54cea995084d3d..14a7d6c5610388 100644 --- a/lldb/unittests/Interpreter/CMakeLists.txt +++ b/lldb/unittests/Interpreter/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(InterpreterTests TestCommandPaths.cpp + TestCommandObjectExpression.cpp TestCompletion.cpp TestOptionArgParser.cpp TestOptions.cpp diff --git a/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp new file mode 100644 index 00000000000000..b0cf22d3cf0903 --- /dev/null +++ b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp @@ -0,0 +1,34 @@ +#include "lldb/Expression/DiagnosticManager.h" +#include "lldb/Utility/StreamString.h" +#include "gtest/gtest.h" + +namespace lldb_private { +std::string RenderDiagnosticDetails(Stream &stream, + std::optional<uint16_t> offset_in_command, + bool multiline, + llvm::ArrayRef<DiagnosticDetail> details); +} + +using namespace lldb_private; +using namespace lldb; +using llvm::StringRef; +namespace { +class ErrorDisplayTest : public ::testing::Test {}; +} // namespace + +static std::string Render(std::vector<DiagnosticDetail> details) { + StreamString stream; + RenderDiagnosticDetails(stream, 0, true, details); + return stream.GetData(); +} + +TEST_F(ErrorDisplayTest, RenderStatus) { + DiagnosticDetail::SourceLocation inline_loc; + inline_loc.in_user_input = true; + { + std::string result = + Render({DiagnosticDetail{inline_loc, eSeverityError, "foo", ""}}); + ASSERT_TRUE(StringRef(result).contains("error:")); + ASSERT_TRUE(StringRef(result).contains("foo")); + } +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
