https://github.com/adrian-prantl created https://github.com/llvm/llvm-project/pull/112109
This allows IDEs to render LLDB expression diagnostics to their liking without relying on characterprecise ASCII art from LLDB. It is exposed as a versioned SBStructuredData object, since it is expected that this may need to be tweaked based on actual usage. >From cf7ea7aa4d458ad82c39afb2b0879ec32a88a2db Mon Sep 17 00:00:00 2001 From: Adrian Prantl <apra...@apple.com> Date: Fri, 11 Oct 2024 19:27:37 -0700 Subject: [PATCH] [lldb] Expose structured command diagnostics via the SBAPI. This allows IDEs to render LLDB expression diagnostics to their liking without relying on characterprecise ASCII art from LLDB. It is exposed as a versioned SBStructuredData object, since it is expected that this may need to be tweaked based on actual usage. --- lldb/include/lldb/API/SBCommandReturnObject.h | 3 +- lldb/include/lldb/API/SBStructuredData.h | 2 + .../lldb/Interpreter/CommandReturnObject.h | 13 ++- lldb/source/API/SBCommandReturnObject.cpp | 11 +++ .../Commands/CommandObjectExpression.cpp | 29 +------ .../source/Interpreter/CommandInterpreter.cpp | 23 +++--- .../Interpreter/CommandReturnObject.cpp | 82 +++++++++++++++---- .../diagnostics/TestExprDiagnostics.py | 25 ++++++ 8 files changed, 128 insertions(+), 60 deletions(-) diff --git a/lldb/include/lldb/API/SBCommandReturnObject.h b/lldb/include/lldb/API/SBCommandReturnObject.h index f96384a4710b16..f0a4738cc346e4 100644 --- a/lldb/include/lldb/API/SBCommandReturnObject.h +++ b/lldb/include/lldb/API/SBCommandReturnObject.h @@ -45,13 +45,14 @@ class LLDB_API SBCommandReturnObject { const char *GetOutput(); const char *GetError(); + SBStructuredData GetErrorData(); #ifndef SWIG LLDB_DEPRECATED_FIXME("Use PutOutput(SBFile) or PutOutput(FileSP)", "PutOutput(SBFile)") size_t PutOutput(FILE *fh); #endif - +\ size_t PutOutput(SBFile file); size_t PutOutput(FileSP BORROWED); diff --git a/lldb/include/lldb/API/SBStructuredData.h b/lldb/include/lldb/API/SBStructuredData.h index fc6e1ec95c7b86..c309e28a190691 100644 --- a/lldb/include/lldb/API/SBStructuredData.h +++ b/lldb/include/lldb/API/SBStructuredData.h @@ -9,6 +9,7 @@ #ifndef LLDB_API_SBSTRUCTUREDDATA_H #define LLDB_API_SBSTRUCTUREDDATA_H +#include "SBCommandReturnObject.h" #include "lldb/API/SBDefines.h" #include "lldb/API/SBModule.h" #include "lldb/API/SBScriptObject.h" @@ -110,6 +111,7 @@ class SBStructuredData { protected: friend class SBAttachInfo; + friend class SBCommandReturnObject; friend class SBLaunchInfo; friend class SBDebugger; friend class SBTarget; diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index eda841869ba432..a491a6c1535b11 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -13,6 +13,7 @@ #include "lldb/Utility/DiagnosticsRendering.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StreamTee.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-private.h" #include "llvm/ADT/StringRef.h" @@ -31,7 +32,7 @@ class CommandReturnObject { ~CommandReturnObject() = default; /// Format any inline diagnostics with an indentation of \c indent. - llvm::StringRef GetInlineDiagnosticString(unsigned indent); + std::string GetInlineDiagnosticString(unsigned indent); llvm::StringRef GetOutputString() { lldb::StreamSP stream_sp(m_out_stream.GetStreamAtIndex(eStreamStringIndex)); @@ -40,7 +41,13 @@ class CommandReturnObject { return llvm::StringRef(); } - llvm::StringRef GetErrorString(); + /// Return the errors as a string. + /// + /// If \c with_diagnostics is true, all diagnostics are also + /// rendered into the string. Otherwise the expectation is that they + /// are fetched with \ref GetInlineDiagnosticString(). + std::string GetErrorString(bool with_diagnostics = true); + StructuredData::ObjectSP GetErrorData(); Stream &GetOutputStream() { // Make sure we at least have our normal string stream output stream @@ -168,7 +175,6 @@ class CommandReturnObject { StreamTee m_out_stream; StreamTee m_err_stream; std::vector<DiagnosticDetail> m_diagnostics; - StreamString m_diag_stream; std::optional<uint16_t> m_diagnostic_indent; lldb::ReturnStatus m_status = lldb::eReturnStatusStarted; @@ -178,6 +184,7 @@ class CommandReturnObject { /// If true, then the input handle from the debugger will be hooked up. bool m_interactive = true; + bool m_colors; }; } // namespace lldb_private diff --git a/lldb/source/API/SBCommandReturnObject.cpp b/lldb/source/API/SBCommandReturnObject.cpp index a94eff75ffcb9e..9df8aa48b99366 100644 --- a/lldb/source/API/SBCommandReturnObject.cpp +++ b/lldb/source/API/SBCommandReturnObject.cpp @@ -11,6 +11,8 @@ #include "lldb/API/SBError.h" #include "lldb/API/SBFile.h" #include "lldb/API/SBStream.h" +#include "lldb/API/SBStructuredData.h" +#include "lldb/Core/StructuredDataImpl.h" #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Utility/ConstString.h" #include "lldb/Utility/Instrumentation.h" @@ -96,6 +98,15 @@ const char *SBCommandReturnObject::GetError() { return output.AsCString(/*value_if_empty*/ ""); } +SBStructuredData SBCommandReturnObject::GetErrorData() { + LLDB_INSTRUMENT_VA(this); + + StructuredData::ObjectSP data(ref().GetErrorData()); + SBStructuredData sb_data; + sb_data.m_impl_up->SetObjectSP(data); + return sb_data; +} + size_t SBCommandReturnObject::GetOutputSize() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index 8effb1a5988370..441392aa7ba019 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -485,35 +485,8 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr, result.SetStatus(eReturnStatusSuccessFinishResult); } else { - // Retrieve the diagnostics. - std::vector<DiagnosticDetail> details; - llvm::consumeError(llvm::handleErrors( - result_valobj_sp->GetError().ToError(), - [&](DiagnosticError &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 { - 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); + result.SetError(result_valobj_sp->GetError().ToError()); } } } else { diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 19bb420f2116dc..3ce715f9e5563f 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -2636,20 +2636,18 @@ void CommandInterpreter::HandleCommands(const StringList &commands, } if (!success || !tmp_result.Succeeded()) { - llvm::StringRef error_msg = tmp_result.GetErrorString(); + std::string error_msg = tmp_result.GetErrorString(); if (error_msg.empty()) error_msg = "<unknown error>.\n"; if (options.GetStopOnError()) { - result.AppendErrorWithFormat( - "Aborting reading of commands after command #%" PRIu64 - ": '%s' failed with %s", - (uint64_t)idx, cmd, error_msg.str().c_str()); + result.AppendErrorWithFormatv("Aborting reading of commands after " + "command #{0}: '{1}' failed with {2}", + (uint64_t)idx, cmd, error_msg); m_debugger.SetAsyncExecution(old_async_execution); return; } else if (options.GetPrintResults()) { - result.AppendMessageWithFormat( - "Command #%" PRIu64 " '%s' failed with %s", (uint64_t)idx + 1, cmd, - error_msg.str().c_str()); + result.AppendMessageWithFormatv("Command #{0} '{1}' failed with {1}", + (uint64_t)idx + 1, cmd, error_msg); } } @@ -3187,11 +3185,12 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) || io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors)) { // Display any inline diagnostics first. - if (!result.GetImmediateErrorStream() && - GetDebugger().GetShowInlineDiagnostics()) { + bool inline_diagnostics = !result.GetImmediateErrorStream() && + GetDebugger().GetShowInlineDiagnostics(); + if (inline_diagnostics) { unsigned prompt_len = m_debugger.GetPrompt().size(); if (auto indent = result.GetDiagnosticIndent()) { - llvm::StringRef diags = + std::string diags = result.GetInlineDiagnosticString(prompt_len + *indent); PrintCommandOutput(io_handler, diags, true); } @@ -3207,7 +3206,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, // Now emit the command error text from the command we just executed. if (!result.GetImmediateErrorStream()) { - llvm::StringRef error = result.GetErrorString(); + std::string error = result.GetErrorString(!inline_diagnostics); PrintCommandOutput(io_handler, error, false); } } diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 28f76dc0c40f94..8cd798f62f775e 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -42,7 +42,7 @@ static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) { } CommandReturnObject::CommandReturnObject(bool colors) - : m_out_stream(colors), m_err_stream(colors), m_diag_stream(colors) {} + : m_out_stream(colors), m_err_stream(colors), m_colors(colors) {} void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) { SetStatus(eReturnStatusFailed); @@ -123,30 +123,80 @@ void CommandReturnObject::SetError(llvm::Error error) { } } -llvm::StringRef +std::string CommandReturnObject::GetInlineDiagnosticString(unsigned indent) { - RenderDiagnosticDetails(m_diag_stream, indent, true, m_diagnostics); + StreamString diag_stream(m_colors); + RenderDiagnosticDetails(diag_stream, indent, true, m_diagnostics); // Duplex the diagnostics to the secondary stream (but not inlined). - if (auto stream_sp = m_err_stream.GetStreamAtIndex(eStreamStringIndex)) + if (auto stream_sp = m_err_stream.GetStreamAtIndex(eImmediateStreamIndex)) RenderDiagnosticDetails(*stream_sp, std::nullopt, false, m_diagnostics); - // Clear them so GetErrorData() doesn't render them again. - m_diagnostics.clear(); - return m_diag_stream.GetString(); + return diag_stream.GetString().str(); } -llvm::StringRef CommandReturnObject::GetErrorString() { - // Diagnostics haven't been fetched; render them now (not inlined). - if (!m_diagnostics.empty()) { - RenderDiagnosticDetails(GetErrorStream(), std::nullopt, false, - m_diagnostics); - m_diagnostics.clear(); - } +std::string CommandReturnObject::GetErrorString(bool with_diagnostics) { + StreamString stream(m_colors); + if (with_diagnostics) + RenderDiagnosticDetails(stream, std::nullopt, false, m_diagnostics); lldb::StreamSP stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex)); if (stream_sp) - return std::static_pointer_cast<StreamString>(stream_sp)->GetString(); - return llvm::StringRef(); + stream << std::static_pointer_cast<StreamString>(stream_sp)->GetString(); + return stream.GetString().str(); +} + +StructuredData::ObjectSP CommandReturnObject::GetErrorData() { + auto make_array = []() { return std::make_unique<StructuredData::Array>(); }; + auto make_bool = [](bool b) { + return std::make_unique<StructuredData::Boolean>(b); + }; + auto make_dict = []() { + return std::make_unique<StructuredData::Dictionary>(); + }; + auto make_int = [](unsigned i) { + return std::make_unique<StructuredData::Float>(i); + }; + auto make_string = [](llvm::StringRef s) { + return std::make_unique<StructuredData::String>(s); + }; + auto dict_up = make_dict(); + dict_up->AddItem("version", make_int(1)); + auto array_up = make_array(); + for (const DiagnosticDetail &diag : m_diagnostics) { + auto detail_up = make_dict(); + if (auto &sloc = diag.source_location) { + auto sloc_up = make_dict(); + sloc_up->AddItem("file", make_string(sloc->file.GetPath())); + sloc_up->AddItem("line", make_int(sloc->line)); + sloc_up->AddItem("length", make_int(sloc->length)); + sloc_up->AddItem("hidden", make_bool(sloc->hidden)); + sloc_up->AddItem("in_user_input", make_bool(sloc->in_user_input)); + detail_up->AddItem("source_location", std::move(sloc_up)); + } + llvm::StringRef severity = "unknown"; + switch (diag.severity) { + case lldb::eSeverityError: + severity = "error"; + break; + case lldb::eSeverityWarning: + severity = "warning"; + break; + case lldb::eSeverityInfo: + severity = "note"; + break; + } + detail_up->AddItem("severity", make_string(severity)); + detail_up->AddItem("message", make_string(diag.message)); + detail_up->AddItem("rendered", make_string(diag.rendered)); + array_up->AddItem(std::move(detail_up)); + } + dict_up->AddItem("details", std::move(array_up)); + if (auto stream_sp = m_err_stream.GetStreamAtIndex(eStreamStringIndex)) { + auto text = std::static_pointer_cast<StreamString>(stream_sp)->GetString(); + if (!text.empty()) + dict_up->AddItem("text", make_string(text)); + } + return dict_up; } // Similar to AppendError, but do not prepend 'Status: ' to message, and don't diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py index c63065a3f345a9..6d5b42a5d7eb94 100644 --- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py +++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py @@ -234,3 +234,28 @@ def check(input_ref): ] ) check(["expression --\na\n+\nb", "error: <user", "a", "error: <user", "b"]) + + def test_command_expr_sbdata(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 + ) + interp = self.dbg.GetCommandInterpreter() + cro = lldb.SBCommandReturnObject() + interp.HandleCommand("expression -- a+b", cro) + diags = cro.GetErrorData() + self.assertEqual(diags.GetValueForKey("version").GetFloatValue(), 1.0) + details = diags.GetValueForKey("details") + diag = details.GetItemAtIndex(0) + self.assertEqual(str(diag.GetValueForKey("severity")), "error") + self.assertIn("undeclared identifier 'a'", str(diag.GetValueForKey("message"))) + self.assertIn("user expression", str(diag.GetValueForKey("rendered"))) + sloc = diag.GetValueForKey("source_location") + self.assertIn("user expression", str(sloc.GetValueForKey("file"))) + self.assertFalse(sloc.GetValueForKey("hidden").GetBooleanValue()) + self.assertTrue(sloc.GetValueForKey("in_user_input").GetBooleanValue()) + diag = details.GetItemAtIndex(1) + self.assertIn("undeclared identifier 'b'", str(diag.GetValueForKey("message"))) + _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits