[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Fri Dec 20 12:26:37 PST 2024


https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/120784

>From 3113275ebe67a970ec01c380153fc6b9814ee939 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Fri, 20 Dec 2024 10:35:47 -0800
Subject: [PATCH] [lldb] Expose structured errors in SBError

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
---
 lldb/include/lldb/API/SBError.h               |  5 ++
 lldb/include/lldb/API/SBStructuredData.h      |  1 +
 .../lldb/Utility/DiagnosticsRendering.h       | 14 +++-
 lldb/include/lldb/Utility/Status.h            |  6 ++
 lldb/source/API/SBError.cpp                   | 14 ++++
 .../Interpreter/CommandReturnObject.cpp       | 52 +------------
 lldb/source/Utility/DiagnosticsRendering.cpp  | 40 ++++++++++
 lldb/source/Utility/Status.cpp                | 24 ++++++
 .../diagnostics/TestExprDiagnostics.py        | 75 ++++++++++++-------
 .../API/commands/frame/var/TestFrameVar.py    | 15 +++-
 10 files changed, 161 insertions(+), 85 deletions(-)

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



More information about the lldb-commits mailing list