Author: Adrian Prantl Date: 2024-12-20T13:02:54-08:00 New Revision: d9cc37fea7b02954079ca59e8f7f28cffacc7e9e
URL: https://github.com/llvm/llvm-project/commit/d9cc37fea7b02954079ca59e8f7f28cffacc7e9e DIFF: https://github.com/llvm/llvm-project/commit/d9cc37fea7b02954079ca59e8f7f28cffacc7e9e.diff LOG: [lldb] Expose structured errors in SBError (#120784) Building on top of previous work that exposed expression diagnostics via SBCommandReturnObject, this patch generalizes the support to expose any SBError as machine-readable structured data. One use-case of this is to allow IDEs to better visualize expression diagnostics. rdar://139997604 Added: Modified: lldb/include/lldb/API/SBError.h lldb/include/lldb/API/SBStructuredData.h lldb/include/lldb/Utility/DiagnosticsRendering.h lldb/include/lldb/Utility/Status.h lldb/source/API/SBError.cpp lldb/source/Interpreter/CommandReturnObject.cpp lldb/source/Utility/DiagnosticsRendering.cpp lldb/source/Utility/Status.cpp lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py lldb/test/API/commands/frame/var/TestFrameVar.py Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h index 9f55f92131c06e..58b45ebd793af5 100644 --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -44,8 +44,13 @@ class LLDB_API SBError { bool Success() const; + /// Get the error code. uint32_t GetError() const; + /// Get the error in machine-readable form. Particularly useful for + /// compiler diagnostics. + SBStructuredData GetErrorData() const; + lldb::ErrorType GetType() const; void SetError(uint32_t err, lldb::ErrorType type); diff --git a/lldb/include/lldb/API/SBStructuredData.h b/lldb/include/lldb/API/SBStructuredData.h index c0d214a7374c65..f96e169f236edd 100644 --- a/lldb/include/lldb/API/SBStructuredData.h +++ b/lldb/include/lldb/API/SBStructuredData.h @@ -115,6 +115,7 @@ class SBStructuredData { friend class SBLaunchInfo; friend class SBDebugger; friend class SBFrame; + friend class SBError; friend class SBTarget; friend class SBProcess; friend class SBThread; diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h b/lldb/include/lldb/Utility/DiagnosticsRendering.h index 33aded602e3a77..dd33d671c24a5f 100644 --- a/lldb/include/lldb/Utility/DiagnosticsRendering.h +++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h @@ -57,6 +57,13 @@ struct DiagnosticDetail { std::string rendered; }; +StructuredData::ObjectSP Serialize(llvm::ArrayRef<DiagnosticDetail> details); + +void RenderDiagnosticDetails(Stream &stream, + std::optional<uint16_t> offset_in_command, + bool show_inline, + llvm::ArrayRef<DiagnosticDetail> details); + class DiagnosticError : public llvm::ErrorInfo<DiagnosticError, CloneableECError> { public: @@ -64,12 +71,11 @@ class DiagnosticError DiagnosticError(std::error_code ec) : ErrorInfo(ec) {} lldb::ErrorType GetErrorType() const override; virtual llvm::ArrayRef<DiagnosticDetail> GetDetails() const = 0; + StructuredData::ObjectSP GetAsStructuredData() const override { + return Serialize(GetDetails()); + } static char ID; }; -void RenderDiagnosticDetails(Stream &stream, - std::optional<uint16_t> offset_in_command, - bool show_inline, - llvm::ArrayRef<DiagnosticDetail> details); } // namespace lldb_private #endif diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h index b8993dff3bb18a..212282cca1f3ed 100644 --- a/lldb/include/lldb/Utility/Status.h +++ b/lldb/include/lldb/Utility/Status.h @@ -10,6 +10,7 @@ #define LLDB_UTILITY_STATUS_H #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringRef.h" @@ -38,6 +39,7 @@ class CloneableError CloneableError() : ErrorInfo() {} virtual std::unique_ptr<CloneableError> Clone() const = 0; virtual lldb::ErrorType GetErrorType() const = 0; + virtual StructuredData::ObjectSP GetAsStructuredData() const = 0; static char ID; }; @@ -49,6 +51,7 @@ class CloneableECError std::error_code convertToErrorCode() const override { return EC; } void log(llvm::raw_ostream &OS) const override { OS << EC.message(); } lldb::ErrorType GetErrorType() const override; + virtual StructuredData::ObjectSP GetAsStructuredData() const override; static char ID; protected: @@ -183,6 +186,9 @@ class Status { /// NULL otherwise. const char *AsCString(const char *default_error_str = "unknown error") const; + /// Get the error in machine-readable form. + StructuredData::ObjectSP GetAsStructuredData() const; + /// Clear the object state. /// /// Reverts the state of this object to contain a generic success value and diff --git a/lldb/source/API/SBError.cpp b/lldb/source/API/SBError.cpp index 31964931649db3..aab4ddd3181dd7 100644 --- a/lldb/source/API/SBError.cpp +++ b/lldb/source/API/SBError.cpp @@ -9,6 +9,8 @@ #include "lldb/API/SBError.h" #include "Utils.h" #include "lldb/API/SBStream.h" +#include "lldb/API/SBStructuredData.h" +#include "lldb/Core/StructuredDataImpl.h" #include "lldb/Utility/Instrumentation.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/VASPrintf.h" @@ -97,6 +99,18 @@ uint32_t SBError::GetError() const { return err; } +SBStructuredData SBError::GetErrorData() const { + LLDB_INSTRUMENT_VA(this); + + SBStructuredData sb_data; + if (!m_opaque_up) + return sb_data; + + StructuredData::ObjectSP data(m_opaque_up->GetAsStructuredData()); + sb_data.m_impl_up->SetObjectSP(data); + return sb_data; +} + ErrorType SBError::GetType() const { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 2776efbb5ee36d..b99b2bc7b36ce4 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -169,57 +169,7 @@ std::string CommandReturnObject::GetErrorString(bool with_diagnostics) { } 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::UnsignedInteger>(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; + return Serialize(m_diagnostics); } // Similar to AppendError, but do not prepend 'Status: ' to message, and don't diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp index f5aa27baadfef8..368e2199b749fb 100644 --- a/lldb/source/Utility/DiagnosticsRendering.cpp +++ b/lldb/source/Utility/DiagnosticsRendering.cpp @@ -20,6 +20,46 @@ lldb::ErrorType DiagnosticError::GetErrorType() const { return lldb::eErrorTypeExpression; } +StructuredData::ObjectSP Serialize(llvm::ArrayRef<DiagnosticDetail> details) { + auto make_array = []() { return std::make_unique<StructuredData::Array>(); }; + auto make_dict = []() { + return std::make_unique<StructuredData::Dictionary>(); + }; + auto dict_up = make_dict(); + dict_up->AddIntegerItem("version", 1u); + auto array_up = make_array(); + for (const DiagnosticDetail &diag : details) { + auto detail_up = make_dict(); + if (auto &sloc = diag.source_location) { + auto sloc_up = make_dict(); + sloc_up->AddStringItem("file", sloc->file.GetPath()); + sloc_up->AddIntegerItem("line", sloc->line); + sloc_up->AddIntegerItem("length", sloc->length); + sloc_up->AddBooleanItem("hidden", sloc->hidden); + sloc_up->AddBooleanItem("in_user_input", 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->AddStringItem("severity", severity); + detail_up->AddStringItem("message", diag.message); + detail_up->AddStringItem("rendered", diag.rendered); + array_up->AddItem(std::move(detail_up)); + } + dict_up->AddItem("details", std::move(array_up)); + return dict_up; +} + static llvm::raw_ostream &PrintSeverity(Stream &stream, lldb::Severity severity) { llvm::HighlightColor color; diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp index 5757935fb86228..49dd469d20bd5e 100644 --- a/lldb/source/Utility/Status.cpp +++ b/lldb/source/Utility/Status.cpp @@ -252,6 +252,30 @@ lldb::ErrorType Win32Error::GetErrorType() const { return lldb::eErrorTypeWin32; } +StructuredData::ObjectSP Status::GetAsStructuredData() const { + auto dict_up = std::make_unique<StructuredData::Dictionary>(); + auto array_up = std::make_unique<StructuredData::Array>(); + llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) { + if (error.isA<CloneableError>()) + array_up->AddItem( + static_cast<const CloneableError &>(error).GetAsStructuredData()); + else + array_up->AddStringItem(error.message()); + }); + dict_up->AddIntegerItem("version", 1u); + dict_up->AddIntegerItem("type", (unsigned)GetType()); + dict_up->AddItem("errors", std::move(array_up)); + return dict_up; +} + +StructuredData::ObjectSP CloneableECError::GetAsStructuredData() const { + auto dict_up = std::make_unique<StructuredData::Dictionary>(); + dict_up->AddIntegerItem("version", 1u); + dict_up->AddIntegerItem("error_code", EC.value()); + dict_up->AddStringItem("message", message()); + return dict_up; +} + ErrorType Status::GetType() const { ErrorType result = eErrorTypeInvalid; llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) { diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py index 0806776aa6eb03..59e759352ce063 100644 --- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py +++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py @@ -207,37 +207,56 @@ def test_command_expr_sbdata(self): (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( self, "// Break here", self.main_source_spec ) + + def check_error(diags): + # Version. + version = diags.GetValueForKey("version") + self.assertEqual(version.GetIntegerValue(), 1) + + details = diags.GetValueForKey("details") + + # Detail 1/2: undeclared 'a' + diag = details.GetItemAtIndex(0) + + severity = diag.GetValueForKey("severity") + message = diag.GetValueForKey("message") + rendered = diag.GetValueForKey("rendered") + sloc = diag.GetValueForKey("source_location") + filename = sloc.GetValueForKey("file") + hidden = sloc.GetValueForKey("hidden") + in_user_input = sloc.GetValueForKey("in_user_input") + + self.assertEqual(str(severity), "error") + self.assertIn("undeclared identifier 'a'", str(message)) + # The rendered string should contain the source file. + self.assertIn("user expression", str(rendered)) + self.assertIn("user expression", str(filename)) + self.assertFalse(hidden.GetBooleanValue()) + self.assertTrue(in_user_input.GetBooleanValue()) + + # Detail 1/2: undeclared 'b' + diag = details.GetItemAtIndex(1) + message = diag.GetValueForKey("message") + self.assertIn("undeclared identifier 'b'", str(message)) + + # Test diagnostics in CommandReturnObject interp = self.dbg.GetCommandInterpreter() cro = lldb.SBCommandReturnObject() interp.HandleCommand("expression -- a+b", cro) diags = cro.GetErrorData() - # Version. - version = diags.GetValueForKey("version") - self.assertEqual(version.GetIntegerValue(), 1) + check_error(diags) - details = diags.GetValueForKey("details") - - # Detail 1/2: undeclared 'a' - diag = details.GetItemAtIndex(0) - - severity = diag.GetValueForKey("severity") - message = diag.GetValueForKey("message") - rendered = diag.GetValueForKey("rendered") - sloc = diag.GetValueForKey("source_location") - filename = sloc.GetValueForKey("file") - hidden = sloc.GetValueForKey("hidden") - in_user_input = sloc.GetValueForKey("in_user_input") - - self.assertEqual(str(severity), "error") - self.assertIn("undeclared identifier 'a'", str(message)) - # The rendered string should contain the source file. - self.assertIn("user expression", str(rendered)) - self.assertIn("user expression", str(filename)) - self.assertFalse(hidden.GetBooleanValue()) - self.assertTrue(in_user_input.GetBooleanValue()) - - # Detail 1/2: undeclared 'b' - diag = details.GetItemAtIndex(1) - message = diag.GetValueForKey("message") - self.assertIn("undeclared identifier 'b'", str(message)) + # Test diagnostics in SBError + frame = thread.GetSelectedFrame() + value = frame.EvaluateExpression("a+b") + error = value.GetError() + self.assertTrue(error.Fail()) + self.assertEquals(error.GetType(), lldb.eErrorTypeExpression) + data = error.GetErrorData() + version = data.GetValueForKey("version") + self.assertEqual(version.GetIntegerValue(), 1) + err_ty = data.GetValueForKey("type") + self.assertEqual(err_ty.GetIntegerValue(), lldb.eErrorTypeExpression) + diags = data.GetValueForKey("errors").GetItemAtIndex(0) + check_error(diags) diff --git a/lldb/test/API/commands/frame/var/TestFrameVar.py b/lldb/test/API/commands/frame/var/TestFrameVar.py index 92e47eb45f5ca5..7211cade5c7c85 100644 --- a/lldb/test/API/commands/frame/var/TestFrameVar.py +++ b/lldb/test/API/commands/frame/var/TestFrameVar.py @@ -113,12 +113,23 @@ def check_frame_variable_errors(self, thread, error_strings): frame = thread.GetFrameAtIndex(0) var_list = frame.GetVariables(True, True, False, True) self.assertEqual(var_list.GetSize(), 0) - api_error = var_list.GetError().GetCString() + api_error = var_list.GetError() + api_error_str = api_error.GetCString() for s in error_strings: self.assertIn(s, command_error) for s in error_strings: - self.assertIn(s, api_error) + self.assertIn(s, api_error_str) + + # Check the structured error data. + data = api_error.GetErrorData() + version = data.GetValueForKey("version") + self.assertEqual(version.GetIntegerValue(), 1) + err_ty = data.GetValueForKey("type") + self.assertEqual(err_ty.GetIntegerValue(), lldb.eErrorTypeGeneric) + message = str(data.GetValueForKey("errors").GetItemAtIndex(0)) + for s in error_strings: + self.assertIn(s, message) @skipIfRemote @skipUnlessDarwin _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits