[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.

Author: adrian-prantl

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);

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;

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

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);
 

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 {

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);
     }
   }

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

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")))
+