[lldb] DRAFT - Add Status::Detail type to hold information from diagnostics

lldb/include/lldb/Expression/DiagnosticManager.h

@@ -10,6 +10,7 @@
 #define LLDB_EXPRESSION_DIAGNOSTICMANAGER_H
 
 #include "lldb/lldb-defines.h"
+#include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-types.h"
 
 #include "llvm/ADT/STLExtras.h"
@@ -20,20 +21,6 @@
 
 namespace lldb_private {
 
-enum DiagnosticOrigin {
-  eDiagnosticOriginUnknown = 0,
-  eDiagnosticOriginLLDB,
-  eDiagnosticOriginClang,
-  eDiagnosticOriginSwift,
-  eDiagnosticOriginLLVM
-};
-
-enum DiagnosticSeverity {
-  eDiagnosticSeverityError,
-  eDiagnosticSeverityWarning,
-  eDiagnosticSeverityRemark
-};
-
 const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX;
 
 class Diagnostic {

lldb/include/lldb/Interpreter/CommandReturnObject.h

@@ -162,6 +162,7 @@ class CommandReturnObject {
   StreamTee m_err_stream;
 
   lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+  Status m_error_status;
 
   bool m_did_change_process_state = false;
   bool m_suppress_immediate_output = false;

lldb/include/lldb/Utility/Status.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_UTILITY_STATUS_H
 #define LLDB_UTILITY_STATUS_H
 
+#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
@@ -114,7 +115,7 @@ class Status {
   ///     The error type enumeration value.
   lldb::ErrorType GetType() const;
 
-  void SetExpressionError(lldb::ExpressionResults, const char *mssg);
+  void SetExpressionError(lldb::ExpressionResults, const char *mssg = nullptr);
 
   int SetExpressionErrorWithFormat(lldb::ExpressionResults, const char *format,
                                    ...) __attribute__((format(printf, 3, 4)));
@@ -180,19 +181,62 @@ class Status {
   ///     success (non-erro), \b false otherwise.
   bool Success() const;
 
+  struct Detail;
+  void AddDetail(Status::Detail detail);
+  void SetErrorDetails(DiagnosticManager &diagnostic_manager);
+  std::vector<Status::Detail> GetDetails() const;
+
 protected:
   /// Member variables
   ValueType m_code = 0; ///< Status code as an integer value.
   lldb::ErrorType m_type =
       lldb::eErrorTypeInvalid;  ///< The type of the above error code.
   mutable std::string m_string; ///< A string representation of the error code.
+  std::vector<Status::Detail> m_status_details;
+  mutable std::string m_string_with_details;
+
 private:
   explicit Status(const llvm::formatv_object_base &payload) {
     SetErrorToGenericError();
     m_string = payload.str();
   }
 };
 
+struct Status::Detail {
+private:
+  std::vector<std::string> m_message_lines;
+  DiagnosticSeverity m_message_type;
+  DiagnosticOrigin m_message_origin;
+
+  mutable std::optional<std::string> m_message;
+
+  static std::string StringForSeverity(DiagnosticSeverity severity);
+
+  std::vector<std::string>
+  GetMessageLinesFromDiagnostic(Diagnostic *diagnostic);
+
+public:
+  Detail(const std::unique_ptr<Diagnostic> &diagnostic)
+      : m_message_lines(GetMessageLinesFromDiagnostic(diagnostic.get())),
+        m_message_type(diagnostic->GetSeverity()),
+        m_message_origin(diagnostic->getKind()) {}
+
+  Detail(const Detail &other)
+      : m_message_lines(other.m_message_lines),
+        m_message_type(other.m_message_type),
+        m_message_origin(other.m_message_origin) {}
+
+  Detail &operator=(const Detail &rhs) {
+    m_message_lines = rhs.m_message_lines;
+    m_message_type = rhs.m_message_type;
+    m_message_origin = rhs.m_message_origin;
+    return *this;
+  }
+
+  std::string GetMessage() const;
+  std::vector<std::string> GetMessageLines() const;
+  DiagnosticSeverity GetType() const { return m_message_type; }
+};
 } // namespace lldb_private
 
 namespace llvm {

lldb/include/lldb/lldb-private-enumerations.h

@@ -234,6 +234,20 @@ enum LoadDependentFiles {
   eLoadDependentsNo,
 };
 
+enum DiagnosticOrigin {
+  eDiagnosticOriginUnknown = 0,
+  eDiagnosticOriginLLDB,
+  eDiagnosticOriginClang,
+  eDiagnosticOriginSwift,
+  eDiagnosticOriginLLVM
+};
+
+enum DiagnosticSeverity {
+  eDiagnosticSeverityError,
+  eDiagnosticSeverityWarning,
+  eDiagnosticSeverityRemark
+};
+
 inline std::string GetStatDescription(lldb_private::StatisticKind K) {
    switch (K) {
    case StatisticKind::ExpressionSuccessful:

lldb/source/Breakpoint/BreakpointLocation.cpp

@@ -247,8 +247,6 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
 
   error.Clear();
 
-  DiagnosticManager diagnostics;
-
   if (condition_hash != m_condition_hash || !m_user_expression_sp ||
       !m_user_expression_sp->IsParseCacheable() ||
       !m_user_expression_sp->MatchesContext(exe_ctx)) {
@@ -269,12 +267,14 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
       return true;
     }
 
-    if (!m_user_expression_sp->Parse(diagnostics, exe_ctx,
-                                     eExecutionPolicyOnlyWhenNeeded, true,
-                                     false)) {
-      error.SetErrorStringWithFormat(
-          "Couldn't parse conditional expression:\n%s",
-          diagnostics.GetString().c_str());
+    DiagnosticManager diagnostic_manager;
+
+    bool success = m_user_expression_sp->Parse(diagnostic_manager, exe_ctx,
+                                               eExecutionPolicyOnlyWhenNeeded,
+                                               true, false);
+    if (!success) {
+      error.SetErrorString("Couldn't parse conditional expression:");
+      error.SetErrorDetails(diagnostic_manager);
       m_user_expression_sp.reset();
       return true;
     }
@@ -296,12 +296,13 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
 
   Status expr_error;
 
-  diagnostics.Clear();
+  DiagnosticManager diagnostic_manager;
 
   ExpressionVariableSP result_variable_sp;
 
-  ExpressionResults result_code = m_user_expression_sp->Execute(
-      diagnostics, exe_ctx, options, m_user_expression_sp, result_variable_sp);
+  ExpressionResults result_code =
+      m_user_expression_sp->Execute(diagnostic_manager, exe_ctx, options,
+                                    m_user_expression_sp, result_variable_sp);
 
   bool ret;
 
@@ -331,8 +332,8 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
     }
   } else {
     ret = false;
-    error.SetErrorStringWithFormat("Couldn't execute expression:\n%s",
-                                   diagnostics.GetString().c_str());
+    error.SetErrorStringWithFormat("Couldn't execute expression:");
+    error.SetErrorDetails(diagnostic_manager);
   }
 
   return ret;

lldb/source/Expression/UserExpression.cpp

@@ -325,17 +325,16 @@ 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 (diagnostic_manager.Diagnostics().empty())
+        msg += "expression failed to parse (no further compiler diagnostics)";
+
+      if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+          !fixed_expression->empty()) {
+        msg += "\nfixed expression suggested:\n  ";
+        msg += *fixed_expression;
       }
       error.SetExpressionError(execution_results, msg.c_str());
+      error.SetErrorDetails(diagnostic_manager);
     }
   }
 
@@ -378,9 +377,10 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
         if (!diagnostic_manager.Diagnostics().size())
           error.SetExpressionError(
               execution_results, "expression failed to execute, unknown error");
-        else
-          error.SetExpressionError(execution_results,
-                                   diagnostic_manager.GetString().c_str());
+        else {
+          error.SetExpressionError(execution_results);
+          error.SetErrorDetails(diagnostic_manager);
+        }
       } else {
         if (expr_result) {
           result_valobj_sp = expr_result->GetValueObject();

lldb/source/Expression/UtilityFunction.cpp

@@ -87,25 +87,25 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
     return nullptr;
   }
   if (m_caller_up) {
-    DiagnosticManager diagnostics;
+    DiagnosticManager diagnostic_manager;
 
     unsigned num_errors =
-        m_caller_up->CompileFunction(thread_to_use_sp, diagnostics);
+        m_caller_up->CompileFunction(thread_to_use_sp, diagnostic_manager);
     if (num_errors) {
-      error.SetErrorStringWithFormat(
-          "Error compiling %s caller function: \"%s\".",
-          m_function_name.c_str(), diagnostics.GetString().c_str());
+      error.SetErrorStringWithFormat("Error compiling %s caller function:",
+                                     m_function_name.c_str());
+      error.SetErrorDetails(diagnostic_manager);
       m_caller_up.reset();
       return nullptr;
     }
 
-    diagnostics.Clear();
+    diagnostic_manager.Clear();
     ExecutionContext exe_ctx(process_sp);
 
-    if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostics)) {
-      error.SetErrorStringWithFormat(
-          "Error inserting caller function for %s: \"%s\".",
-          m_function_name.c_str(), diagnostics.GetString().c_str());
+    if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostic_manager)) {
+      error.SetErrorStringWithFormat("Error inserting caller function for %s:",
+                                     m_function_name.c_str());
+      error.SetErrorDetails(diagnostic_manager);
       m_caller_up.reset();
       return nullptr;
     }

lldb/source/Interpreter/CommandReturnObject.cpp

@@ -109,8 +109,15 @@ void CommandReturnObject::AppendError(llvm::StringRef in_string) {
 
 void CommandReturnObject::SetError(const Status &error,
                                    const char *fallback_error_cstr) {
-  if (error.Fail())
-    AppendError(error.AsCString(fallback_error_cstr));
+  m_error_status = error;
+  if (m_error_status.Fail()) {
+    std::vector<Status::Detail> details = m_error_status.GetDetails();
+    if (!details.empty()) {
+      for (Status::Detail detail : details)
+        AppendError(detail.GetMessage());
+    } else
+      AppendError(error.AsCString(fallback_error_cstr));
+  }
 }
 
 void CommandReturnObject::SetError(llvm::Error error) {

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

@@ -1424,7 +1424,7 @@ lldb_private::Status ClangExpressionParser::PrepareForExecution(
           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.SetErrorDetails(install_diags);
             err.SetErrorString(ErrMsg);
             return err;
           }
@@ -1505,8 +1505,8 @@ lldb_private::Status ClangExpressionParser::RunStaticInitializers(
             exe_ctx, call_static_initializer, options, execution_errors);
 
     if (results != lldb::eExpressionCompleted) {
-      err.SetErrorStringWithFormat("couldn't run static initializer: %s",
-                                   execution_errors.GetString().c_str());
+      err.SetErrorString("couldn't run static initializer: ");
+      err.SetErrorDetails(execution_errors);
       return err;
     }
   }

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

@@ -851,9 +851,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
                                                  func_args_addr,
                                                  arguments,
                                                  diagnostics)) {
-    error.SetErrorStringWithFormat(
-        "dlopen error: could not write function arguments: %s",
-        diagnostics.GetString().c_str());
+    error.SetErrorString("dlopen error: could not write function arguments: ");
+    error.SetErrorDetails(diagnostics);
     return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -893,9 +892,9 @@ 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.SetErrorStringWithFormat(
-        "dlopen error: failed executing dlopen wrapper function: %s",
-        diagnostics.GetString().c_str());
+    error.SetErrorString(
+        "dlopen error: failed executing dlopen wrapper function: ");
+    error.SetErrorDetails(diagnostics);
     return LLDB_INVALID_IMAGE_TOKEN;
   }
 

lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp

@@ -331,8 +331,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
   diagnostics.Clear();
   if (!invocation->WriteFunctionArguments(context, injected_parameters,
                                           parameters, diagnostics)) {
-    error.SetErrorStringWithFormat("LoadLibrary error: unable to write function parameters: %s",
-                                   diagnostics.GetString().c_str());
+    error.SetErrorString(
+        "LoadLibrary error: unable to write function parameters: ");
+    error.SetErrorDetails(diagnostics);
     return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -372,8 +373,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
       invocation->ExecuteFunction(context, &injected_parameters, options,
                                   diagnostics, value);
   if (result != eExpressionCompleted) {
-    error.SetErrorStringWithFormat("LoadLibrary error: failed to execute LoadLibrary helper: %s",
-                                   diagnostics.GetString().c_str());
+    error.SetErrorString(
+        "LoadLibrary error: failed to execute LoadLibrary helper: ");
+    error.SetErrorDetails(diagnostics);
     return LLDB_INVALID_IMAGE_TOKEN;
   }