[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 10 17:11:21 PDT 2024


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

>From d6d55a4d6f806f260e875dc29fdf7481d44fa1b1 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 4 Sep 2024 12:50:37 -0700
Subject: [PATCH 1/2] [lldb] Change the implementation of Status to store an
 llvm::Error (NFC)

Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. Where possible, APIs should be
refactored to avoid Status. The only legitimate long-term uses of
Status are objects that need to store an error for a long time (which
should be questioned as a design decision, too).

This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked
with a call to Status::clone(). Every other API can and should be
refactored to use llvm::Expected. In the end Status should only be
used in very few places.

Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.

Implementation notes:

This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:

       (eErrorTypeInvalid)
       eErrorTypeGeneric      llvm::StringError
       eErrorTypePOSIX        llvm::ECError
       eErrorTypeMachKernel   MachKernelError
       eErrorTypeExpression   llvm::ErrorList<ExpressionError>
       eErrorTypeWin32        Win32Error
---
 lldb/include/lldb/Utility/Status.h            |  83 +++++-
 .../Python/PythonDataObjects.cpp              |   5 +-
 lldb/source/Utility/Status.cpp                | 244 ++++++++++++------
 3 files changed, 242 insertions(+), 90 deletions(-)

diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h
index 795c830b965173..517724aa94fae4 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -28,6 +28,67 @@ 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
+/// operation.
+class CloneableError
+    : public llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase> {
+public:
+  using llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase>::ErrorInfo;
+  CloneableError() : ErrorInfo() {}
+  virtual std::unique_ptr<CloneableError> Clone() const = 0;
+  static char ID;
+};
+
+/// Common base class for all error-code errors.
+class CloneableECError
+    : public llvm::ErrorInfo<CloneableECError, CloneableError> {
+public:
+  using llvm::ErrorInfo<CloneableECError, CloneableError>::ErrorInfo;
+  CloneableECError(std::error_code ec) : ErrorInfo() {}
+  std::error_code convertToErrorCode() const override { return EC; }
+  void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
+  static char ID;
+
+protected:
+  std::error_code EC;
+};
+
+/// FIXME: Move these declarations closer to where they're used.
+class MachKernelError
+    : public llvm::ErrorInfo<MachKernelError, CloneableECError> {
+public:
+  using llvm::ErrorInfo<MachKernelError, CloneableECError>::ErrorInfo;
+  MachKernelError(std::error_code ec) : ErrorInfo(ec) {}
+  std::string message() const override;
+  std::unique_ptr<CloneableError> Clone() const override;
+  static char ID;
+};
+
+class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
+public:
+  using llvm::ErrorInfo<Win32Error, CloneableECError>::ErrorInfo;
+  Win32Error(std::error_code ec, const llvm::Twine &msg = {}) : ErrorInfo(ec) {}
+  std::string message() const override;
+  std::unique_ptr<CloneableError> Clone() const override;
+  static char ID;
+};
+
+class ExpressionError
+    : public llvm::ErrorInfo<ExpressionError, CloneableECError> {
+public:
+  using llvm::ErrorInfo<ExpressionError, CloneableECError>::ErrorInfo;
+  ExpressionError(std::error_code ec, std::string msg = {})
+      : ErrorInfo(ec), m_string(msg) {}
+  std::unique_ptr<CloneableError> Clone() const override;
+  std::string message() const override { return m_string; }
+  static char ID;
+
+protected:
+  std::string m_string;
+};
+
 /// \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
@@ -100,9 +161,7 @@ class Status {
   }
 
   static Status FromExpressionError(lldb::ExpressionResults result,
-                                    std::string msg) {
-    return Status(result, lldb::eErrorTypeExpression, msg);
-  }
+                                    std::string msg);
 
   /// Set the current error to errno.
   ///
@@ -115,6 +174,7 @@ class Status {
   const Status &operator=(Status &&);
   /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
   static Status FromError(llvm::Error error);
+
   /// FIXME: Replace this with a takeError() method.
   llvm::Error ToError() const;
   /// Don't call this function in new code. Instead, redesign the API
@@ -149,12 +209,20 @@ class Status {
 
   /// Access the error value.
   ///
+  /// If the internally stored \ref llvm::Error is an \ref
+  /// llvm::ErrorList then this returns the error value of the first
+  /// error.
+  ///  
   /// \return
   ///     The error value.
   ValueType GetError() const;
 
   /// Access the error type.
   ///
+  /// If the internally stored \ref llvm::Error is an \ref
+  /// llvm::ErrorList then this returns the error value of the first
+  /// error.
+  ///
   /// \return
   ///     The error type enumeration value.
   lldb::ErrorType GetType() const;
@@ -170,12 +238,9 @@ class Status {
   bool Success() const;
 
 protected:
-  Status(llvm::Error error);
-  /// Status code as an integer value.
-  ValueType m_code = 0;
-  /// The type of the above error code.
-  lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
-  /// A string representation of the error code.
+  Status(llvm::Error error) : m_error(std::move(error)) {}
+  llvm::Error m_error;
+  /// TODO: Replace this with just callling toString(m_error).
   mutable std::string m_string;
 };
 
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 24cf3430006329..79e1db9b54e590 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -973,6 +973,9 @@ PythonException::PythonException(const char *caller) {
         PyErr_Clear();
       }
       Py_XDECREF(repr);
+      Py_INCREF(m_exception);
+      if (m_traceback)
+        Py_INCREF(m_traceback);
     } else {
       PyErr_Clear();
     }
@@ -993,8 +996,8 @@ void PythonException::Restore() {
 }
 
 PythonException::~PythonException() {
-  Py_XDECREF(m_exception_type);
   Py_XDECREF(m_exception);
+  Py_XDECREF(m_exception_type);
   Py_XDECREF(m_traceback);
   Py_XDECREF(m_repr_bytes);
 }
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 4af3af5fba0185..733f7cfeef6765 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -8,6 +8,8 @@
 
 #include "lldb/Utility/Status.h"
 
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -37,48 +39,74 @@ class raw_ostream;
 using namespace lldb;
 using namespace lldb_private;
 
-Status::Status() {}
+char CloneableError::ID;
+char CloneableECError::ID;
+char MachKernelError::ID;
+char Win32Error::ID;
+char ExpressionError::ID;
+
+namespace {
+/// A std::error_code category for eErrorTypeGeneric.
+class GenericCategory : public std::error_category {
+  const char *name() const override { return "LLDBGenericCategory"; }
+  std::string message(int __ev) const override { return "generic LLDB error"; };
+};
+GenericCategory &generic_category() {
+  static GenericCategory g_generic_category;
+  return g_generic_category;
+}
+
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+  const char *name() const override { return "LLDBExpressionCategory"; }
+  std::string message(int __ev) const override {
+    return ExpressionResultAsCString(
+        static_cast<lldb::ExpressionResults>(__ev));
+  };
+};
+ExpressionCategory &expression_category() {
+  static ExpressionCategory g_expression_category;
+  return g_expression_category;
+}
+} // namespace
+
+Status::Status() : m_error(llvm::Error::success()) {}
+
+static llvm::Error ErrorFromEnums(Status::ValueType err, ErrorType type,
+                                  std::string msg) {
+  switch (type) {
+  case eErrorTypeMachKernel:
+    return llvm::make_error<MachKernelError>(
+        std::error_code(err, std::system_category()));
+  case eErrorTypePOSIX:
+    return llvm::errorCodeToError(
+        std::error_code(err, std::generic_category()));
+  case eErrorTypeWin32:
+    return llvm::make_error<Win32Error>(
+        std::error_code(err, std::system_category()));
+  default:
+    return llvm::createStringError(std::move(msg),
+                                   std::error_code(err, generic_category()));
+  }
+}
 
 Status::Status(ValueType err, ErrorType type, std::string msg)
-    : m_code(err), m_type(type), m_string(std::move(msg)) {}
+    : m_error(ErrorFromEnums(err, type, msg)) {}
 
-// This logic is confusing because c++ calls the traditional (posix) errno codes
+// This logic is confusing because C++ calls the traditional (posix) errno codes
 // "generic errors", while we use the term "generic" to mean completely
 // arbitrary (text-based) errors.
 Status::Status(std::error_code EC)
-    : m_code(EC.value()),
-      m_type(EC.category() == std::generic_category() ? eErrorTypePOSIX
-                                                      : eErrorTypeGeneric),
-      m_string(EC.message()) {}
+    : m_error(!EC ? llvm::Error::success() : llvm::errorCodeToError(EC)) {}
 
 Status::Status(std::string err_str)
-    : m_code(LLDB_GENERIC_ERROR), m_type(eErrorTypeGeneric),
-      m_string(std::move(err_str)) {}
-
-Status::Status(llvm::Error error) {
-  if (!error) {
-    Clear();
-    return;
-  }
+    : m_error(
+          llvm::createStringError(llvm::inconvertibleErrorCode(), err_str)) {}
 
-  // if the error happens to be a errno error, preserve the error code
-  error = llvm::handleErrors(
-      std::move(error), [&](std::unique_ptr<llvm::ECError> e) -> llvm::Error {
-        std::error_code ec = e->convertToErrorCode();
-        if (ec.category() == std::generic_category()) {
-          m_code = ec.value();
-          m_type = ErrorType::eErrorTypePOSIX;
-          return llvm::Error::success();
-        }
-        return llvm::Error(std::move(e));
-      });
-
-  // Otherwise, just preserve the message
-  if (error) {
-    m_code = LLDB_GENERIC_ERROR;
-    m_type = eErrorTypeGeneric;
-    m_string = llvm::toString(std::move(error));
-  }
+const Status &Status::operator=(Status &&other) {
+  Clear();
+  m_error = std::move(other.m_error);
+  return *this;
 }
 
 Status Status::FromErrorStringWithFormat(const char *format, ...) {
@@ -94,25 +122,36 @@ Status Status::FromErrorStringWithFormat(const char *format, ...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) { return Status(std::move(error)); }
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+                                   std::string msg) {
+  return Status(llvm::make_error<ExpressionError>(
+      std::error_code(result, expression_category()), msg));
+}
 
-llvm::Error Status::ToError() const {
-  if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(const llvm::Error &error) {
+  std::vector<std::unique_ptr<llvm::ErrorInfoBase>> info;
+  llvm::visitErrors(error, [&](const llvm::ErrorInfoBase &error) {
+    if (error.isA<CloneableError>())
+      info.push_back(static_cast<const CloneableError *>(&error)->Clone());
+    else
+      info.push_back(std::make_unique<llvm::StringError>(
+          error.message(), error.convertToErrorCode(), true));
+  });
+  if (info.size() == 0)
     return llvm::Error::success();
-  if (m_type == ErrorType::eErrorTypePOSIX)
-    return llvm::errorCodeToError(
-        std::error_code(m_code, std::generic_category()));
-  return llvm::createStringError(AsCString());
+  llvm::Error e(std::move(info.front()));
+  for (auto it = std::next(info.begin()); it != info.end(); ++it)
+    e = llvm::joinErrors(std::move(e), llvm::Error(std::move(*it)));
+  return e;
 }
 
-Status::~Status() = default;
+Status Status::FromError(llvm::Error error) { return Status(std::move(error)); }
+
+llvm::Error Status::ToError() const { return CloneError(m_error); }
 
-const Status &Status::operator=(Status &&other) {
-  m_code = other.m_code;
-  m_type = other.m_type;
-  m_string = std::move(other.m_string);
-  return *this;
-}
+Status::~Status() { llvm::consumeError(std::move(m_error)); }
 
 #ifdef _WIN32
 static std::string RetrieveWin32ErrorString(uint32_t error_code) {
@@ -140,6 +179,33 @@ static std::string RetrieveWin32ErrorString(uint32_t error_code) {
 }
 #endif
 
+std::string MachKernelError::message() const {
+#if defined(__APPLE__)
+  if (const char *s = ::mach_error_string(convertToErrorCode().value()))
+    return s;
+#endif
+  return "MachKernelError";
+}
+
+std::string Win32Error::message() const {
+#if defined(_WIN32)
+  return RetrieveWin32ErrorString(convertToErrorCode().value());
+#endif
+  return "Win32Error";
+}
+
+std::unique_ptr<CloneableError> MachKernelError::Clone() const {
+  return std::make_unique<MachKernelError>(convertToErrorCode());
+}
+
+std::unique_ptr<CloneableError> Win32Error::Clone() const {
+  return std::make_unique<Win32Error>(convertToErrorCode());
+}
+
+std::unique_ptr<CloneableError> ExpressionError::Clone() const {
+  return std::make_unique<ExpressionError>(convertToErrorCode(), message());
+}
+
 // Get the error value as a NULL C string. The error string will be fetched and
 // cached on demand. The cached error string value will remain until the error
 // value is changed or cleared.
@@ -147,29 +213,8 @@ const char *Status::AsCString(const char *default_error_str) const {
   if (Success())
     return nullptr;
 
-  if (m_string.empty()) {
-    switch (m_type) {
-    case eErrorTypeMachKernel:
-#if defined(__APPLE__)
-      if (const char *s = ::mach_error_string(m_code))
-        m_string.assign(s);
-#endif
-      break;
-
-    case eErrorTypePOSIX:
-      m_string = llvm::sys::StrError(m_code);
-      break;
-
-    case eErrorTypeWin32:
-#if defined(_WIN32)
-      m_string = RetrieveWin32ErrorString(m_code);
-#endif
-      break;
+  m_string = llvm::toStringWithoutConsuming(m_error);
 
-    default:
-      break;
-    }
-  }
   if (m_string.empty()) {
     if (default_error_str)
       m_string.assign(default_error_str);
@@ -181,29 +226,68 @@ const char *Status::AsCString(const char *default_error_str) const {
 
 // Clear the error and any cached error string that it might contain.
 void Status::Clear() {
-  m_code = 0;
-  m_type = eErrorTypeInvalid;
-  m_string.clear();
+  if (m_error)
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::API), std::move(m_error),
+                    "dropping error {0}");
+  m_error = llvm::Error::success();
+  llvm::consumeError(std::move(m_error));
 }
 
-// Access the error value.
-Status::ValueType Status::GetError() const { return m_code; }
+Status::ValueType Status::GetError() const {
+  Status::ValueType result = 0;
+  llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) {
+    // Return the first only.
+    if (result)
+      return;
+    std::error_code ec = error.convertToErrorCode();
+    if (ec.category() == std::generic_category() ||
+        ec.category() == generic_category() ||
+        ec.category() == expression_category())
+      result = ec.value();
+    else
+      result = 0xff;
+  });
+  return result;
+}
 
 // Access the error type.
-ErrorType Status::GetType() const { return m_type; }
+ErrorType Status::GetType() const {
+  ErrorType result = eErrorTypeInvalid;
+  llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) {
+    // Return the first only.
+    if (result != eErrorTypeInvalid)
+      return;
+    if (error.isA<MachKernelError>())
+      result = eErrorTypeMachKernel;
+    else if (error.isA<Win32Error>())
+      result = eErrorTypeWin32;
+    else if (error.isA<ExpressionError>())
+      result = eErrorTypeExpression;
+    else if (error.convertToErrorCode().category() == std::generic_category())
+      result = eErrorTypePOSIX;
+    else if (error.convertToErrorCode().category() == generic_category() ||
+             error.convertToErrorCode() == llvm::inconvertibleErrorCode())
+      result = eErrorTypeGeneric;
+    else
+      result = eErrorTypeInvalid;
+  });
+  return result;
+}
 
-// Returns true if this object contains a value that describes an error or
-// otherwise non-success result.
-bool Status::Fail() const { return m_code != 0; }
+bool Status::Fail() const {
+  // Note that this does not clear the checked flag in
+  // m_error. Otherwise we'd need to make this thread-safe.
+  return m_error.isA<llvm::ErrorInfoBase>();
+}
 
 Status Status::FromErrno() {
   // Update the error value to be "errno" and update the type to be "POSIX".
-  return Status(errno, eErrorTypePOSIX);
+  return Status(llvm::errorCodeToError(llvm::errnoAsErrorCode()));
 }
 
 // Returns true if the error code in this object is considered a successful
 // return value.
-bool Status::Success() const { return m_code == 0; }
+bool Status::Success() const { return !Fail(); }
 
 void llvm::format_provider<lldb_private::Status>::format(
     const lldb_private::Status &error, llvm::raw_ostream &OS,

>From 542229ecd139e47e535a83f5e91ca63570087961 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 28 Aug 2024 10:04:33 -0700
Subject: [PATCH 2/2] [lldb] Store expression evaluator diagnostics in an
 llvm::Error (NFC)

This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938

This patch is preparatory patch for improving the rendering of
expression evaluator diagnostics. Currently diagnostics are rendered
into a string and the command interpreter layer then textually parses
words like "error:" to (sometimes) color the output accordingly. In
order to enable user interfaces to do better with diagnostics, we need
to store them in a machine-readable fromat. This patch does this by
adding a new llvm::Error kind wrapping a DiagnosticDetail struct that
is used when the error type is eErrorTypeExpression. Multiple
diagnostics are modeled using llvm::ErrorList.

Right now the extra information is not used by the CommandInterpreter,
this will be added in a follow-up patch!
---
 .../lldb/Expression/DiagnosticManager.h       | 88 ++++++++++++++-----
 lldb/include/lldb/Utility/Status.h            |  5 +-
 lldb/source/Breakpoint/BreakpointLocation.cpp | 10 +--
 lldb/source/Expression/DiagnosticManager.cpp  | 83 +++++++++++++----
 lldb/source/Expression/ExpressionParser.cpp   |  5 +-
 lldb/source/Expression/UserExpression.cpp     | 28 +++---
 lldb/source/Expression/UtilityFunction.cpp    | 16 ++--
 .../ExpressionParser/Clang/ClangDiagnostic.h  |  5 +-
 .../Clang/ClangExpressionParser.cpp           | 57 +++++++++---
 .../Plugins/Platform/POSIX/PlatformPOSIX.cpp  | 10 +--
 .../Platform/Windows/PlatformWindows.cpp      | 10 +--
 lldb/source/Target/Target.cpp                 |  2 +-
 lldb/source/Utility/Status.cpp                |  5 ++
 .../TestModulesCompileError.py                |  2 +-
 .../Expression/DiagnosticManagerTest.cpp      | 12 +--
 15 files changed, 227 insertions(+), 111 deletions(-)

diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..bcf108a4d9d398 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,9 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -20,6 +23,47 @@
 
 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 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 DetailedExpressionError
+    : public llvm::ErrorInfo<DetailedExpressionError, llvm::ECError> {
+  DiagnosticDetail m_detail;
+
+public:
+  using llvm::ErrorInfo<DetailedExpressionError, llvm::ECError>::ErrorInfo;
+  DetailedExpressionError(DiagnosticDetail detail) : m_detail(detail) {}
+  std::string message() const override;
+  static char ID;
+};
+
 enum DiagnosticOrigin {
   eDiagnosticOriginUnknown = 0,
   eDiagnosticOriginLLDB,
@@ -49,37 +93,28 @@ class Diagnostic {
     }
   }
 
-  Diagnostic(llvm::StringRef message, lldb::Severity severity,
-             DiagnosticOrigin origin, uint32_t compiler_id)
-      : m_message(message), m_severity(severity), m_origin(origin),
-        m_compiler_id(compiler_id) {}
-
-  Diagnostic(const Diagnostic &rhs)
-      : m_message(rhs.m_message), m_severity(rhs.m_severity),
-        m_origin(rhs.m_origin), m_compiler_id(rhs.m_compiler_id) {}
+  Diagnostic(DiagnosticOrigin origin, uint32_t compiler_id,
+             DiagnosticDetail detail)
+      : m_origin(origin), m_compiler_id(compiler_id), m_detail(detail) {}
 
   virtual ~Diagnostic() = default;
 
   virtual bool HasFixIts() const { return false; }
 
-  lldb::Severity GetSeverity() const { return m_severity; }
+  lldb::Severity GetSeverity() const { return m_detail.severity; }
 
   uint32_t GetCompilerID() const { return m_compiler_id; }
 
-  llvm::StringRef GetMessage() const { return m_message; }
+  llvm::StringRef GetMessage() const { return m_detail.message; }
+  llvm::Error GetAsError() const;
 
-  void AppendMessage(llvm::StringRef message,
-                     bool precede_with_newline = true) {
-    if (precede_with_newline)
-      m_message.push_back('\n');
-    m_message += message;
-  }
+  void AppendMessage(llvm::StringRef message, bool precede_with_newline = true);
 
 protected:
-  std::string m_message;
-  lldb::Severity m_severity;
   DiagnosticOrigin m_origin;
-  uint32_t m_compiler_id; // Compiler-specific diagnostic ID
+  /// Compiler-specific diagnostic ID.
+  uint32_t m_compiler_id;
+  DiagnosticDetail m_detail;
 };
 
 typedef std::vector<std::unique_ptr<Diagnostic>> DiagnosticList;
@@ -102,10 +137,7 @@ class DiagnosticManager {
 
   void AddDiagnostic(llvm::StringRef message, lldb::Severity severity,
                      DiagnosticOrigin origin,
-                     uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
-    m_diagnostics.emplace_back(
-        std::make_unique<Diagnostic>(message, severity, origin, compiler_id));
-  }
+                     uint32_t compiler_id = LLDB_INVALID_COMPILER_ID);
 
   void AddDiagnostic(std::unique_ptr<Diagnostic> diagnostic) {
     if (diagnostic)
@@ -130,13 +162,21 @@ class DiagnosticManager {
       m_diagnostics.back()->AppendMessage(str);
   }
 
+  /// Copies the diagnostics into an llvm::Error{List}.
+  /// The first diagnostic wraps \c result.
+  llvm::Error GetAsError(lldb::ExpressionResults result) const;
+
+  /// Copies the diagnostics into an llvm::Error, the first diagnostic
+  /// being an llvm::StringError.
+  llvm::Error GetAsError(llvm::Twine msg) const;
+
   // Returns a string containing errors in this format:
   //
   // "error: error text\n
   // warning: warning text\n
   // remark text\n"
   std::string GetString(char separator = '\n');
-
+  
   void Dump(Log *log);
 
   const std::string &GetFixedExpression() { return m_fixed_expression; }
diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h
index 517724aa94fae4..1255af0ab3fb16 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -175,8 +175,11 @@ class Status {
   /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
   static Status FromError(llvm::Error error);
 
-  /// FIXME: Replace this with a takeError() method.
+  /// FIXME: Replace all uses with takeError() instead.
   llvm::Error ToError() const;
+
+  llvm::Error takeError() { return std::move(m_error); }
+  
   /// Don't call this function in new code. Instead, redesign the API
   /// to use llvm::Expected instead of Status.
   Status Clone() const { return Status(ToError()); }
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 8d7364052a006a..fd9248eb0677c5 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -264,9 +264,9 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
     if (!m_user_expression_sp->Parse(diagnostics, exe_ctx,
                                      eExecutionPolicyOnlyWhenNeeded, true,
                                      false)) {
-      error = Status::FromErrorStringWithFormat(
-          "Couldn't parse conditional expression:\n%s",
-          diagnostics.GetString().c_str());
+      error = Status::FromError(
+          diagnostics.GetAsError("Couldn't parse conditional expression:"));
+
       m_user_expression_sp.reset();
       return true;
     }
@@ -324,8 +324,8 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
     }
   } else {
     ret = false;
-    error = Status::FromErrorStringWithFormat(
-        "Couldn't execute expression:\n%s", diagnostics.GetString().c_str());
+    error = Status::FromError(
+        diagnostics.GetAsError("Couldn't execute expression:"));
   }
 
   return ret;
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index a8330138f3d53b..1fff224fd912e9 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -14,22 +14,7 @@
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb_private;
-
-void DiagnosticManager::Dump(Log *log) {
-  if (!log)
-    return;
-
-  std::string str = GetString();
-
-  // GetString() puts a separator after each diagnostic. We want to remove the
-  // last '\n' because log->PutCString will add one for us.
-
-  if (str.size() && str.back() == '\n') {
-    str.pop_back();
-  }
-
-  log->PutCString(str.c_str());
-}
+char DetailedExpressionError::ID;
 
 static const char *StringForSeverity(lldb::Severity severity) {
   switch (severity) {
@@ -44,9 +29,16 @@ static const char *StringForSeverity(lldb::Severity severity) {
   llvm_unreachable("switch needs another case for lldb::Severity enum");
 }
 
+std::string DetailedExpressionError::message() const {
+  std::string str;
+  llvm::raw_string_ostream(str)
+      << StringForSeverity(m_detail.severity) << m_detail.rendered;
+  return str;
+}
+
 std::string DiagnosticManager::GetString(char separator) {
-  std::string ret;
-  llvm::raw_string_ostream stream(ret);
+  std::string str;
+  llvm::raw_string_ostream stream(str);
 
   for (const auto &diagnostic : Diagnostics()) {
     llvm::StringRef severity = StringForSeverity(diagnostic->GetSeverity());
@@ -61,8 +53,51 @@ std::string DiagnosticManager::GetString(char separator) {
       stream << message.drop_front(severity_pos + severity.size());
     stream << separator;
   }
+  return str;
+}
 
-  return ret;
+void DiagnosticManager::Dump(Log *log) {
+  if (!log)
+    return;
+
+  std::string str = GetString();
+
+  // We want to remove the last '\n' because log->PutCString will add
+  // one for us.
+
+  if (str.size() && str.back() == '\n')
+    str.pop_back();
+
+  log->PutString(str);
+}
+
+llvm::Error Diagnostic::GetAsError() const {
+  return llvm::make_error<DetailedExpressionError>(m_detail);
+}
+
+llvm::Error
+DiagnosticManager::GetAsError(lldb::ExpressionResults result) const {
+  llvm::Error diags = Status::FromExpressionError(result, "").takeError();
+  for (const auto &diagnostic : m_diagnostics)
+    diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+  return diags;
+}
+
+llvm::Error
+DiagnosticManager::GetAsError(llvm::Twine msg) const {
+  llvm::Error diags = llvm::createStringError(msg);
+  for (const auto &diagnostic : m_diagnostics)
+    diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+  return diags;
+}
+
+void DiagnosticManager::AddDiagnostic(llvm::StringRef message,
+                                      lldb::Severity severity,
+                                      DiagnosticOrigin origin,
+                                      uint32_t compiler_id) {
+  m_diagnostics.emplace_back(std::make_unique<Diagnostic>(
+      origin, compiler_id,
+      DiagnosticDetail{{}, severity, message.str(), message.str()}));
 }
 
 size_t DiagnosticManager::Printf(lldb::Severity severity, const char *format,
@@ -85,3 +120,13 @@ void DiagnosticManager::PutString(lldb::Severity severity,
     return;
   AddDiagnostic(str, severity, eDiagnosticOriginLLDB);
 }
+
+void Diagnostic::AppendMessage(llvm::StringRef message,
+                               bool precede_with_newline) {
+  if (precede_with_newline) {
+    m_detail.message.push_back('\n');
+    m_detail.rendered.push_back('\n');
+  }
+  m_detail.message += message;
+  m_detail.rendered += message;
+}
diff --git a/lldb/source/Expression/ExpressionParser.cpp b/lldb/source/Expression/ExpressionParser.cpp
index 868556c1c58a57..26bc19874c53cc 100644
--- a/lldb/source/Expression/ExpressionParser.cpp
+++ b/lldb/source/Expression/ExpressionParser.cpp
@@ -63,9 +63,8 @@ ExpressionParser::RunStaticInitializers(IRExecutionUnitSP &execution_unit_sp,
             exe_ctx, call_static_initializer, options, execution_errors);
 
     if (results != eExpressionCompleted) {
-      err = Status::FromErrorStringWithFormat(
-          "couldn't run static initializer: %s",
-          execution_errors.GetString().c_str());
+      err = Status::FromError(
+          execution_errors.GetAsError("couldn't run static initializer:"));
       return err;
     }
   }
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 872f6304f91baa..3e035cfbff567b 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -328,18 +328,20 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
     }
 
     if (!parse_success) {
-      std::string msg;
-      {
-        llvm::raw_string_ostream os(msg);
-        if (!diagnostic_manager.Diagnostics().empty())
-          os << diagnostic_manager.GetString();
-        else
-          os << "expression failed to parse (no further compiler diagnostics)";
-        if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
-            !fixed_expression->empty())
-          os << "\nfixed expression suggested:\n  " << *fixed_expression;
+      if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+          !fixed_expression->empty()) {
+        std::string fixit =
+            "fixed expression suggested:\n  " + *fixed_expression;
+        diagnostic_manager.AddDiagnostic(fixit, lldb::eSeverityInfo,
+                                         eDiagnosticOriginLLDB);
       }
-      error = Status::FromExpressionError(execution_results, msg);
+      if (!diagnostic_manager.Diagnostics().size())
+        error = Status::FromExpressionError(
+            execution_results,
+            "expression failed to parse (no further compiler diagnostics)");
+      else
+        error =
+            Status::FromError(diagnostic_manager.GetAsError(execution_results));
     }
   }
 
@@ -384,8 +386,8 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
           error = Status::FromExpressionError(
               execution_results, "expression failed to execute, unknown error");
         else
-          error = Status::FromExpressionError(execution_results,
-                                              diagnostic_manager.GetString());
+          error = Status::FromError(
+              diagnostic_manager.GetAsError(execution_results));
       } else {
         if (expr_result) {
           result_valobj_sp = expr_result->GetValueObject();
diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp
index 55ebfb8ef342e8..e35c0774815298 100644
--- a/lldb/source/Expression/UtilityFunction.cpp
+++ b/lldb/source/Expression/UtilityFunction.cpp
@@ -83,19 +83,18 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
   m_caller_up.reset(process_sp->GetTarget().GetFunctionCallerForLanguage(
       Language().AsLanguageType(), return_type, impl_code_address,
       arg_value_list, name.c_str(), error));
-  if (error.Fail()) {
-
+  if (error.Fail())
     return nullptr;
-  }
+
   if (m_caller_up) {
     DiagnosticManager diagnostics;
 
     unsigned num_errors =
         m_caller_up->CompileFunction(thread_to_use_sp, diagnostics);
     if (num_errors) {
-      error = Status::FromErrorStringWithFormat(
-          "Error compiling %s caller function: \"%s\".",
-          m_function_name.c_str(), diagnostics.GetString().c_str());
+      error = Status::FromError(diagnostics.GetAsError(
+          "Error compiling " + m_function_name + " caller function:"));
+
       m_caller_up.reset();
       return nullptr;
     }
@@ -104,9 +103,8 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
     ExecutionContext exe_ctx(process_sp);
 
     if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostics)) {
-      error = Status::FromErrorStringWithFormat(
-          "Error inserting caller function for %s: \"%s\".",
-          m_function_name.c_str(), diagnostics.GetString().c_str());
+      error = Status::FromError(diagnostics.GetAsError(
+          "Error inserting " + m_function_name + " caller function:"));
       m_caller_up.reset();
       return nullptr;
     }
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
index 21abd71cc34eeb..c473df808ee8d0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
@@ -29,9 +29,8 @@ class ClangDiagnostic : public Diagnostic {
     return diag->getKind() == eDiagnosticOriginClang;
   }
 
-  ClangDiagnostic(llvm::StringRef message, lldb::Severity severity,
-                  uint32_t compiler_id)
-      : Diagnostic(message, severity, eDiagnosticOriginClang, compiler_id) {}
+  ClangDiagnostic(DiagnosticDetail detail, uint32_t compiler_id)
+      : Diagnostic(eDiagnosticOriginClang, compiler_id, detail) {}
 
   ~ClangDiagnostic() override = default;
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 90f26de939a478..16c708626dbdcd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -26,6 +26,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Frontend/TextDiagnostic.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/Preprocessor.h"
@@ -212,20 +213,20 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
     m_passthrough->HandleDiagnostic(DiagLevel, Info);
     m_os->flush();
 
-    lldb::Severity severity;
+    DiagnosticDetail detail;
     bool make_new_diagnostic = true;
 
     switch (DiagLevel) {
     case DiagnosticsEngine::Level::Fatal:
     case DiagnosticsEngine::Level::Error:
-      severity = lldb::eSeverityError;
+      detail.severity = lldb::eSeverityError;
       break;
     case DiagnosticsEngine::Level::Warning:
-      severity = lldb::eSeverityWarning;
+      detail.severity = lldb::eSeverityWarning;
       break;
     case DiagnosticsEngine::Level::Remark:
     case DiagnosticsEngine::Level::Ignored:
-      severity = lldb::eSeverityInfo;
+      detail.severity = lldb::eSeverityInfo;
       break;
     case DiagnosticsEngine::Level::Note:
       m_manager->AppendMessageToDiagnostic(m_output);
@@ -254,14 +255,46 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
       std::string stripped_output =
           std::string(llvm::StringRef(m_output).trim());
 
-      auto new_diagnostic = std::make_unique<ClangDiagnostic>(
-          stripped_output, severity, Info.getID());
+      // Translate the source location.
+      if (Info.hasSourceManager()) {
+        DiagnosticDetail::SourceLocation loc;
+        clang::SourceManager &sm = Info.getSourceManager();
+        const clang::SourceLocation sloc = Info.getLocation();
+        if (sloc.isValid()) {
+          const clang::FullSourceLoc fsloc(sloc, sm);
+          clang::PresumedLoc PLoc = fsloc.getPresumedLoc(true);
+          StringRef filename =
+              PLoc.isValid() ? PLoc.getFilename() : StringRef{};
+          loc.file = FileSpec(filename);
+          loc.line = fsloc.getSpellingLineNumber();
+          loc.column = fsloc.getSpellingColumnNumber();
+          // A heuristic detecting the #line 1 "<user expression 1>".
+          loc.in_user_input = filename.starts_with("<user expression ");
+          // Find the range of the primary location.
+          for (const auto &range : Info.getRanges()) {
+            if (range.getBegin() == sloc) {
+              // FIXME: This is probably not handling wide characters correctly.
+              unsigned end_col = sm.getSpellingColumnNumber(range.getEnd());
+              if (end_col > loc.column)
+                loc.length = end_col - loc.column;
+              break;
+            }
+          }
+          detail.source_location = loc;
+        }
+      }
+      llvm::SmallString<0> msg;
+      Info.FormatDiagnostic(msg);
+      detail.message = msg.str();
+      detail.rendered = stripped_output;
+      auto new_diagnostic =
+          std::make_unique<ClangDiagnostic>(detail, Info.getID());
 
       // Don't store away warning fixits, since the compiler doesn't have
       // enough context in an expression for the warning to be useful.
       // FIXME: Should we try to filter out FixIts that apply to our generated
       // code, and not the user's expression?
-      if (severity == lldb::eSeverityError)
+      if (detail.severity == lldb::eSeverityError)
         AddAllFixIts(new_diagnostic.get(), Info);
 
       m_manager->AddDiagnostic(std::move(new_diagnostic));
@@ -1505,13 +1538,9 @@ lldb_private::Status ClangExpressionParser::DoPrepareForExecution(
               new ClangDynamicCheckerFunctions();
 
           DiagnosticManager install_diags;
-          if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx)) {
-            std::string ErrMsg = "couldn't install checkers: " + toString(std::move(Err));
-            if (install_diags.Diagnostics().size())
-              ErrMsg = ErrMsg + "\n" + install_diags.GetString().c_str();
-            err = Status(ErrMsg);
-            return err;
-          }
+          if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx))
+            return Status::FromError(
+                install_diags.GetAsError("couldn't install checkers:"));
 
           process->SetDynamicCheckers(dynamic_checkers);
 
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 31315e46ca168d..f0609596ded731 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -863,9 +863,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
                                                  func_args_addr,
                                                  arguments,
                                                  diagnostics)) {
-    error = Status::FromErrorStringWithFormat(
-        "dlopen error: could not write function arguments: %s",
-        diagnostics.GetString().c_str());
+    error = Status::FromError(diagnostics.GetAsError(
+        "dlopen error: could not write function arguments:" ));
     return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -906,9 +905,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
   ExpressionResults results = do_dlopen_function->ExecuteFunction(
       exe_ctx, &func_args_addr, options, diagnostics, return_value);
   if (results != eExpressionCompleted) {
-    error = Status::FromErrorStringWithFormat(
-        "dlopen error: failed executing dlopen wrapper function: %s",
-        diagnostics.GetString().c_str());
+    error = Status::FromError(diagnostics.GetAsError(
+        "dlopen error: failed executing dlopen wrapper function:"));
     return LLDB_INVALID_IMAGE_TOKEN;
   }
   
diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
index 7352d6f33f2174..fa39d93a91297c 100644
--- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -341,9 +341,8 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
   diagnostics.Clear();
   if (!invocation->WriteFunctionArguments(context, injected_parameters,
                                           parameters, diagnostics)) {
-    error = Status::FromErrorStringWithFormat(
-        "LoadLibrary error: unable to write function parameters: %s",
-        diagnostics.GetString().c_str());
+    error = Status::FromError(diagnostics.GetAsError(
+        "LoadLibrary error: unable to write function parameters:"));
     return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -384,9 +383,8 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
       invocation->ExecuteFunction(context, &injected_parameters, options,
                                   diagnostics, value);
   if (result != eExpressionCompleted) {
-    error = Status::FromErrorStringWithFormat(
-        "LoadLibrary error: failed to execute LoadLibrary helper: %s",
-        diagnostics.GetString().c_str());
+    error = Status::FromError(diagnostics.GetAsError(
+        "LoadLibrary error: failed to execute LoadLibrary helper:"));
     return LLDB_INVALID_IMAGE_TOKEN;
   }
 
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index f1c378b968d2ba..775a9eb2ab2988 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2605,7 +2605,7 @@ Target::CreateUtilityFunction(std::string expression, std::string name,
 
   DiagnosticManager diagnostics;
   if (!utility_fn->Install(diagnostics, exe_ctx))
-    return llvm::createStringError(diagnostics.GetString());
+    return diagnostics.GetAsError("Could not install utility function:");
 
   return std::move(utility_fn);
 }
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 733f7cfeef6765..83b2587c0b0da5 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -214,6 +214,11 @@ const char *Status::AsCString(const char *default_error_str) const {
     return nullptr;
 
   m_string = llvm::toStringWithoutConsuming(m_error);
+  // FIXME: Workaround for ErrorList[ExpressionError, ...].
+  while (!m_string.empty() && m_string[0] == '\n')
+    m_string = std::string(m_string.data() + 1, m_string.size() - 1);
+  if (!m_string.empty() && m_string[m_string.size()-1] != '\n')
+    m_string += '\n';
 
   if (m_string.empty()) {
     if (default_error_str)
diff --git a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
index 620b6e44fc852a..36e302be2525b5 100644
--- a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
+++ b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
@@ -21,7 +21,7 @@ def test(self):
             "expr @import LLDBTestModule",
             error=True,
             substrs=[
-                "module.h:4:1: use of undeclared identifier 'syntax_error_for_lldb_to_find'",
+                "module.h:4:1: error: use of undeclared identifier 'syntax_error_for_lldb_to_find'",
                 "syntax_error_for_lldb_to_find // comment that tests source printing",
                 "could not build module 'LLDBTestModule'",
             ],
diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
index 05fe7c164d6818..596baedd728ffa 100644
--- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -19,8 +19,8 @@ class FixItDiag : public Diagnostic {
 
 public:
   FixItDiag(llvm::StringRef msg, bool has_fixits)
-      : Diagnostic(msg, lldb::eSeverityError,
-                   DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id),
+      : Diagnostic(DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id,
+                   DiagnosticDetail{{}, lldb::eSeverityError, msg.str(), {}}),
         m_has_fixits(has_fixits) {}
   bool HasFixIts() const override { return m_has_fixits; }
 };
@@ -30,8 +30,8 @@ namespace {
 class TextDiag : public Diagnostic {
 public:
   TextDiag(llvm::StringRef msg, lldb::Severity severity)
-      : Diagnostic(msg, severity, DiagnosticOrigin::eDiagnosticOriginLLDB,
-                   custom_diag_id) {}
+      : Diagnostic(DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id,
+                   DiagnosticDetail{{}, severity, msg.str(), {}}) {}
 };
 } // namespace
 
@@ -42,8 +42,8 @@ TEST(DiagnosticManagerTest, AddDiagnostic) {
   std::string msg = "foo bar has happened";
   lldb::Severity severity = lldb::eSeverityError;
   DiagnosticOrigin origin = DiagnosticOrigin::eDiagnosticOriginLLDB;
-  auto diag =
-      std::make_unique<Diagnostic>(msg, severity, origin, custom_diag_id);
+  auto diag = std::make_unique<Diagnostic>(
+      origin, custom_diag_id, DiagnosticDetail{{}, severity, msg, {}});
   mgr.AddDiagnostic(std::move(diag));
   EXPECT_EQ(1U, mgr.Diagnostics().size());
   const Diagnostic *got = mgr.Diagnostics().front().get();



More information about the lldb-commits mailing list