Support inline diagnostics in CommandReturnObject

and implement them for dwim-print (a.k.a. `p`) and the command option
parser (in one place thus far) as an example.

The next step will be to expose them as structured data in
SBCommandReturnObject.

Author: adrian-prantl

lldb/include/lldb/Expression/DiagnosticManager.h

@@ -12,6 +12,7 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Utility/DiagnosticsRendering.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 
@@ -23,49 +24,22 @@
 
 namespace lldb_private {
 
-/// A compiler-independent representation of a Diagnostic. Expression
-/// evaluation failures often have more than one diagnostic that a UI
-/// layer might want to render differently, for example to colorize
-/// it.
-///
-/// Running example:
-///   (lldb) expr 1+foo
-///   error: <user expression 0>:1:3: use of undeclared identifier 'foo'
-///   1+foo
-///     ^
-struct DiagnosticDetail {
-  struct SourceLocation {
-    FileSpec file;
-    unsigned line = 0;
-    uint16_t column = 0;
-    uint16_t length = 0;
-    bool hidden = false;
-    bool in_user_input = false;
-  };
-  /// Contains {{}, 1, 3, 3, true} in the example above.
-  std::optional<SourceLocation> source_location;
-  /// Contains eSeverityError in the example above.
-  lldb::Severity severity = lldb::eSeverityInfo;
-  /// Contains "use of undeclared identifier 'x'" in the example above.
-  std::string message;
-  /// Contains the fully rendered error message.
-  std::string rendered;
-};
-
 /// An llvm::Error used to communicate diagnostics in Status. Multiple
 /// diagnostics may be chained in an llvm::ErrorList.
 class ExpressionError
-    : public llvm::ErrorInfo<ExpressionError, ExpressionErrorBase> {
+    : public llvm::ErrorInfo<ExpressionError, DiagnosticError> {
   std::string m_message;
   std::vector<DiagnosticDetail> m_details;
 
 public:
   static char ID;
-  using llvm::ErrorInfo<ExpressionError, ExpressionErrorBase>::ErrorInfo;
+  using llvm::ErrorInfo<ExpressionError, DiagnosticError>::ErrorInfo;
   ExpressionError(lldb::ExpressionResults result, std::string msg,
                   std::vector<DiagnosticDetail> details = {});
   std::string message() const override;
-  llvm::ArrayRef<DiagnosticDetail> GetDetails() const { return m_details; }
+  llvm::ArrayRef<DiagnosticDetail> GetDetails() const override {
+    return m_details;
+  }
   std::error_code convertToErrorCode() const override;
   void log(llvm::raw_ostream &OS) const override;
   std::unique_ptr<CloneableError> Clone() const override;

lldb/include/lldb/Interpreter/CommandReturnObject.h

@@ -10,6 +10,7 @@
 #define LLDB_INTERPRETER_COMMANDRETURNOBJECT_H
 
 #include "lldb/Host/StreamFile.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StreamTee.h"
 #include "lldb/lldb-private.h"
@@ -29,19 +30,16 @@ class CommandReturnObject {
 
   ~CommandReturnObject() = default;
 
+  llvm::StringRef GetInlineDiagnosticsData(unsigned indent);
+
   llvm::StringRef GetOutputData() {
     lldb::StreamSP stream_sp(m_out_stream.GetStreamAtIndex(eStreamStringIndex));
     if (stream_sp)
       return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
     return llvm::StringRef();
   }
 
-  llvm::StringRef GetErrorData() {
-    lldb::StreamSP stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex));
-    if (stream_sp)
-      return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
-    return llvm::StringRef();
-  }
+  llvm::StringRef GetErrorData();
 
   Stream &GetOutputStream() {
     // Make sure we at least have our normal string stream output stream
@@ -135,6 +133,14 @@ class CommandReturnObject {
 
   void SetError(llvm::Error error);
 
+  void SetDiagnosticIndent(std::optional<uint16_t> indent) {
+    m_diagnostic_indent = indent;
+  }
+
+  std::optional<uint16_t> GetDiagnosticIndent() const {
+    return m_diagnostic_indent;
+  }
+
   lldb::ReturnStatus GetStatus() const;
 
   void SetStatus(lldb::ReturnStatus status);
@@ -160,6 +166,9 @@ 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;
 

lldb/include/lldb/Utility/Args.h

@@ -38,19 +38,23 @@ class Args {
 
     std::unique_ptr<char[]> ptr;
     char quote = '\0';
+    /// The position of the argument in the original argument string.
+    std::optional<uint16_t> column;
 
     char *data() { return ptr.get(); }
 
   public:
     ArgEntry() = default;
-    ArgEntry(llvm::StringRef str, char quote);
+    ArgEntry(llvm::StringRef str, char quote, std::optional<uint16_t> column);
 
     llvm::StringRef ref() const { return c_str(); }
     const char *c_str() const { return ptr.get(); }
 
     /// Returns true if this argument was quoted in any way.
     bool IsQuoted() const { return quote != '\0'; }
     char GetQuoteChar() const { return quote; }
+    std::optional<uint16_t> GetPos() const { return column; }
+    uint16_t GetLength() const { return ref().size(); }
   };
 
   /// Construct with an option command string.

lldb/include/lldb/Utility/DiagnosticsRendering.h

@@ -0,0 +1,75 @@
+//===-- DiagnosticsRendering.h ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_DIAGNOSTICSRENDERING_H
+#define LLDB_UTILITY_DIAGNOSTICSRENDERING_H
+
+#include "lldb/Utility/Status.h"
+#include "lldb/Utility/Stream.h"
+#include "llvm/Support/WithColor.h"
+
+namespace lldb_private {
+
+/// A compiler-independent representation of an \c
+/// lldb_private::Diagnostic. Expression evaluation failures often
+/// have more than one diagnostic that a UI layer might want to render
+/// differently, for example to colorize it.
+///
+/// Running example:
+///   (lldb) expr 1 + foo
+///   error: <user expression 0>:1:3: use of undeclared identifier 'foo'
+///   1 + foo
+///       ^~~
+struct DiagnosticDetail {
+  /// A source location consisting of a file name and position.
+  struct SourceLocation {
+    /// \c "<user expression 0>" in the example above.
+    FileSpec file;
+    /// \c 1 in the example above.
+    unsigned line = 0;
+    /// \c 5 in the example above.
+    uint16_t column = 0;
+    /// \c 3 in the example above.
+    uint16_t length = 0;
+    /// Whether this source location should be surfaced to the
+    /// user. For example, syntax errors diagnosed in LLDB's
+    /// expression wrapper code have this set to true.
+    bool hidden = false;
+    /// Whether this source location refers to something the user
+    /// typed as part of the command, i.e., if this qualifies for
+    /// inline display, or if the source line would need to be echoed
+    /// again for the message to make sense.
+    bool in_user_input = false;
+  };
+  /// Contains this diagnostic's source location, if applicable.
+  std::optional<SourceLocation> source_location;
+  /// Contains \c eSeverityError in the example above.
+  lldb::Severity severity = lldb::eSeverityInfo;
+  /// Contains "use of undeclared identifier 'foo'" in the example above.
+  std::string message;
+  /// Contains the fully rendered error message, without "error: ",
+  /// but including the source context.
+  std::string rendered;
+};
+
+class DiagnosticError
+    : public llvm::ErrorInfo<DiagnosticError, CloneableECError> {
+public:
+  using llvm::ErrorInfo<DiagnosticError, CloneableECError>::ErrorInfo;
+  DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
+  lldb::ErrorType GetErrorType() const override;
+  virtual llvm::ArrayRef<DiagnosticDetail> GetDetails() const;
+  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

lldb/include/lldb/Utility/Status.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_UTILITY_STATUS_H
 #define LLDB_UTILITY_STATUS_H
 
+#include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
@@ -26,8 +27,6 @@ class raw_ostream;
 
 namespace lldb_private {
 
-const char *ExpressionResultAsCString(lldb::ExpressionResults result);
-
 /// Going a bit against the spirit of llvm::Error,
 /// lldb_private::Status need to store errors long-term and sometimes
 /// copy them. This base class defines an interface for this
@@ -79,15 +78,6 @@ class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
   static char ID;
 };
 
-class ExpressionErrorBase
-    : public llvm::ErrorInfo<ExpressionErrorBase, CloneableECError> {
-public:
-  using llvm::ErrorInfo<ExpressionErrorBase, CloneableECError>::ErrorInfo;
-  ExpressionErrorBase(std::error_code ec) : ErrorInfo(ec) {}
-  lldb::ErrorType GetErrorType() const override;
-  static char ID;
-};
-
 /// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
 ///
 /// This class is designed to be able to hold any error code that can be

lldb/source/Commands/CommandObjectDWIMPrint.cpp

@@ -191,6 +191,15 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
     ExpressionResults expr_result = target.EvaluateExpression(
         expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
+    // Record the position of the expression in the command.
+    std::optional<uint16_t> indent;
+    if (fixed_expression.empty()) {
+      size_t pos = m_original_command.find(expr);
+      if (pos != llvm::StringRef::npos)
+        indent = pos;
+    }
+    result.SetDiagnosticIndent(indent);
+
     // Only mention Fix-Its if the expression evaluator applied them.
     // Compiler errors refer to the final expression after applying Fix-It(s).
     if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {

lldb/source/Commands/CommandObjectExpression.cpp

@@ -7,9 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "CommandObjectExpression.h"
-#include "DiagnosticRendering.h"
 #include "lldb/Core/Debugger.h"
-#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
 #include "lldb/Expression/UserExpression.h"
@@ -22,6 +20,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
@@ -490,7 +489,7 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
         std::vector<DiagnosticDetail> details;
         llvm::consumeError(llvm::handleErrors(
             result_valobj_sp->GetError().ToError(),
-            [&](ExpressionError &error) { details = error.GetDetails(); }));
+            [&](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);

lldb/source/Expression/DiagnosticManager.cpp

@@ -10,6 +10,7 @@
 
 #include "llvm/Support/ErrorHandling.h"
 
+#include "lldb/Utility/ErrorMessages.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -22,8 +23,7 @@ class ExpressionCategory : public std::error_category {
     return "LLDBExpressionCategory";
   }
   std::string message(int __ev) const override {
-    return ExpressionResultAsCString(
-        static_cast<lldb::ExpressionResults>(__ev));
+    return toString(static_cast<lldb::ExpressionResults>(__ev));
   };
 };
 ExpressionCategory &expression_category() {

lldb/source/Expression/FunctionCaller.cpp

@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ValueObject.h"
@@ -24,6 +23,7 @@
 #include "lldb/Target/ThreadPlan.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/ErrorMessages.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -390,7 +390,7 @@ lldb::ExpressionResults FunctionCaller::ExecuteFunction(
       LLDB_LOGF(log,
                 "== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
                 "completed abnormally: %s ==",
-                m_name.c_str(), ExpressionResultAsCString(return_value));
+                m_name.c_str(), toString(return_value).c_str());
     } else {
       LLDB_LOGF(log,
                 "== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "

lldb/source/Expression/LLVMUserExpression.cpp

@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-
 #include "lldb/Expression/LLVMUserExpression.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ValueObjectConstResult.h"
@@ -30,6 +29,7 @@
 #include "lldb/Target/ThreadPlan.h"
 #include "lldb/Target/ThreadPlanCallUserExpression.h"
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/ErrorMessages.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
@@ -237,7 +237,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
     } else if (execution_result != lldb::eExpressionCompleted) {
       diagnostic_manager.Printf(lldb::eSeverityError,
                                 "Couldn't execute function; result was %s",
-                                ExpressionResultAsCString(execution_result));
+                                toString(execution_result).c_str());
       return execution_result;
     }
   }