[lldb] Expose structured command diagnostics via the SBAPI. #112109
+128
−60
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
adrian-prantl
requested review from
medismailben and
jimingham
and removed request for
JDevlieghere
|
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) ChangesThis 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. Full diff: https:/llvm/llvm-project/pull/112109.diff 8 Files Affected:
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")))
+
|
You can test this locally with the following command:darker --check --diff -r c2750807ba2a419425ee90dadda09ad5121517fe...cf7ea7aa4d458ad82c39afb2b0879ec32a88a2db lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.pyView the diff from darker here.--- TestExprDiagnostics.py 2024-10-12 20:37:19.000000 +0000
+++ TestExprDiagnostics.py 2024-10-12 20:41:04.212598 +0000
@@ -256,6 +256,5 @@
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")))
-
|
You can test this locally with the following command:git-clang-format --diff c2750807ba2a419425ee90dadda09ad5121517fe cf7ea7aa4d458ad82c39afb2b0879ec32a88a2db --extensions h,cpp -- lldb/include/lldb/API/SBCommandReturnObject.h lldb/include/lldb/API/SBStructuredData.h lldb/include/lldb/Interpreter/CommandReturnObject.h lldb/source/API/SBCommandReturnObject.cpp lldb/source/Commands/CommandObjectExpression.cpp lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Interpreter/CommandReturnObject.cppView the diff from clang-format here.diff --git a/lldb/include/lldb/API/SBCommandReturnObject.h b/lldb/include/lldb/API/SBCommandReturnObject.h
index f0a4738cc3..e8e20a3f30 100644
--- a/lldb/include/lldb/API/SBCommandReturnObject.h
+++ b/lldb/include/lldb/API/SBCommandReturnObject.h
@@ -52,7 +52,7 @@ public:
"PutOutput(SBFile)")
size_t PutOutput(FILE *fh);
#endif
-\
+
size_t PutOutput(SBFile file);
size_t PutOutput(FileSP BORROWED);
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 8cd798f62f..6855c6fcb6 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -123,8 +123,7 @@ void CommandReturnObject::SetError(llvm::Error error) {
}
}
-std::string
-CommandReturnObject::GetInlineDiagnosticString(unsigned indent) {
+std::string CommandReturnObject::GetInlineDiagnosticString(unsigned indent) {
StreamString diag_stream(m_colors);
RenderDiagnosticDetails(diag_stream, indent, true, m_diagnostics);
// Duplex the diagnostics to the secondary stream (but not inlined).
|
|
|
||
| #ifndef SWIG | ||
| LLDB_DEPRECATED_FIXME("Use PutOutput(SBFile) or PutOutput(FileSP)", | ||
| "PutOutput(SBFile)") | ||
| size_t PutOutput(FILE *fh); | ||
| #endif | ||
|
|
||
| \ |
| @@ -9,6 +9,7 @@ | |||
| #ifndef LLDB_API_SBSTRUCTUREDDATA_H | |||
| #define LLDB_API_SBSTRUCTUREDDATA_H | |||
|
|
|||
| #include "SBCommandReturnObject.h" | |||
There was a problem hiding this comment.
Suggested change
| #include "SBCommandReturnObject.h" | |
| #include "lldb/API/SBCommandReturnObject.h" |
| return std::make_unique<StructuredData::Dictionary>(); | ||
| }; | ||
| auto make_int = [](unsigned i) { | ||
| return std::make_unique<StructuredData::Float>(i); |
There was a problem hiding this comment.
Should this be StructuredData::UnsignedInteger or StructuredData::SignedInteger?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.