[Lldb-commits] [lldb] [lldb] DRAFT - Remove 2nd "error: " and print caret (^) below last input line from the developer. (PR #72150)

Pete Lawrence via lldb-commits lldb-commits at lists.llvm.org
Mon Nov 27 13:43:25 PST 2023


https://github.com/PortalPete updated https://github.com/llvm/llvm-project/pull/72150

>From 047413349901684411fb260a49c996633b7b4dc0 Mon Sep 17 00:00:00 2001
From: Pete Lawrence <plawrence at apple.com>
Date: Mon, 6 Nov 2023 17:16:28 -1000
Subject: [PATCH] Remove secondary "error: " and print diagnostic line with
 caret (^) just below the developer's last command.

---
 .../lldb/Expression/DiagnosticManager.h       | 67 +-----------------
 .../lldb/Interpreter/CommandReturnObject.h    |  4 ++
 lldb/include/lldb/Utility/Status.h            | 70 +++++++++++++++++++
 lldb/source/Expression/UserExpression.cpp     | 14 ++++
 .../source/Interpreter/CommandInterpreter.cpp | 59 +++++++++++++++-
 .../Interpreter/CommandReturnObject.cpp       |  6 ++
 lldb/source/Utility/Status.cpp                |  8 +++
 7 files changed, 160 insertions(+), 68 deletions(-)

diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index df9ba3b245f51e8..5eec90241aa51d8 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -14,80 +14,15 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "lldb/Utility/Status.h"
 
 #include <string>
 #include <vector>
 
 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 {
-  friend class DiagnosticManager;
-
-public:
-  DiagnosticOrigin getKind() const { return m_origin; }
-
-  static bool classof(const Diagnostic *diag) {
-    DiagnosticOrigin kind = diag->getKind();
-    switch (kind) {
-    case eDiagnosticOriginUnknown:
-    case eDiagnosticOriginLLDB:
-    case eDiagnosticOriginLLVM:
-      return true;
-    case eDiagnosticOriginClang:
-    case eDiagnosticOriginSwift:
-      return false;
-    }
-  }
-
-  Diagnostic(llvm::StringRef message, DiagnosticSeverity 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) {}
-
-  virtual ~Diagnostic() = default;
-
-  virtual bool HasFixIts() const { return false; }
-
-  DiagnosticSeverity GetSeverity() const { return m_severity; }
-
-  uint32_t GetCompilerID() const { return m_compiler_id; }
-
-  llvm::StringRef GetMessage() const { return m_message; }
-
-  void AppendMessage(llvm::StringRef message,
-                     bool precede_with_newline = true) {
-    if (precede_with_newline)
-      m_message.push_back('\n');
-    m_message += message;
-  }
-
-protected:
-  std::string m_message;
-  DiagnosticSeverity m_severity;
-  DiagnosticOrigin m_origin;
-  uint32_t m_compiler_id; // Compiler-specific diagnostic ID
-};
-
 typedef std::vector<std::unique_ptr<Diagnostic>> DiagnosticList;
 
 class DiagnosticManager {
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 8c4dcb54d708f08..ec8b09f151fac93 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_INTERPRETER_COMMANDRETURNOBJECT_H
 #define LLDB_INTERPRETER_COMMANDRETURNOBJECT_H
 
+#include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StreamTee.h"
@@ -139,6 +140,8 @@ class CommandReturnObject {
 
   void SetStatus(lldb::ReturnStatus status);
 
+  std::vector<Diagnostic> GetDiagnostics() const;
+
   bool Succeeded() const;
 
   bool HasResult() const;
@@ -162,6 +165,7 @@ class CommandReturnObject {
   StreamTee m_err_stream;
 
   lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+  std::vector<Diagnostic> m_diagnostics;
 
   bool m_did_change_process_state = false;
   bool m_suppress_immediate_output = false;
diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h
index fa5768141fa45df..840177385b9e1a2 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -25,6 +25,71 @@ class raw_ostream;
 }
 
 namespace lldb_private {
+  enum DiagnosticOrigin {
+    eDiagnosticOriginUnknown = 0,
+    eDiagnosticOriginLLDB,
+    eDiagnosticOriginClang,
+    eDiagnosticOriginSwift,
+    eDiagnosticOriginLLVM
+  };
+
+  enum DiagnosticSeverity {
+    eDiagnosticSeverityError,
+    eDiagnosticSeverityWarning,
+    eDiagnosticSeverityRemark
+  };
+
+  class Diagnostic {
+    friend class DiagnosticManager;
+
+  public:
+    DiagnosticOrigin getKind() const { return m_origin; }
+
+    static bool classof(const Diagnostic *diag) {
+      DiagnosticOrigin kind = diag->getKind();
+      switch (kind) {
+        case eDiagnosticOriginUnknown:
+        case eDiagnosticOriginLLDB:
+        case eDiagnosticOriginLLVM:
+          return true;
+        case eDiagnosticOriginClang:
+        case eDiagnosticOriginSwift:
+          return false;
+      }
+    }
+
+    Diagnostic(llvm::StringRef message, DiagnosticSeverity 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) {}
+
+    virtual ~Diagnostic() = default;
+
+    virtual bool HasFixIts() const { return false; }
+
+    DiagnosticSeverity GetSeverity() const { return m_severity; }
+
+    uint32_t GetCompilerID() const { return m_compiler_id; }
+
+    llvm::StringRef GetMessage() const { return m_message; }
+
+    void AppendMessage(llvm::StringRef message,
+                       bool precede_with_newline = true) {
+      if (precede_with_newline)
+        m_message.push_back('\n');
+      m_message += message;
+    }
+
+  protected:
+    std::string m_message;
+    DiagnosticSeverity m_severity;
+    DiagnosticOrigin m_origin;
+    uint32_t m_compiler_id; // Compiler-specific diagnostic ID
+  };
 
 /// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
 ///
@@ -180,12 +245,17 @@ class Status {
   ///     success (non-erro), \b false otherwise.
   bool Success() const;
 
+  void AddDiagnostic(Diagnostic diagnostic);
+
+  std::vector<Diagnostic> GetDiagnostics() 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<Diagnostic> m_diagnostics;
 private:
   explicit Status(const llvm::formatv_object_base &payload) {
     SetErrorToGenericError();
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index c181712a2f0b243..03b42a841020d31 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -278,6 +278,20 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
       user_expression_sp->Parse(diagnostic_manager, exe_ctx, execution_policy,
                                 keep_expression_in_memory, generate_debug_info);
 
+  // Copy diagnostics from the manager to the error (`Status`) instance's
+  // diagnostic vector.
+  {
+    const DiagnosticList &source = diagnostic_manager.Diagnostics();
+
+    if (source.size() >= 1) {
+      std::vector<Diagnostic> destination = error.GetDiagnostics();
+      destination.clear();
+
+      for (auto &entry : source)
+        destination.push_back(*entry);
+    }
+  }
+
   // Calculate the fixed expression always, since we need it for errors.
   std::string tmp_fixed_expression;
   if (fixed_expression == nullptr)
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index e1275ce711fc172..5b29ad32b3365ca 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3071,6 +3071,44 @@ bool CommandInterpreter::EchoCommandNonInteractive(
 
   return true;
 }
+// MARK: - Diagnostic helpers functions
+static std::string BuildCaretLine(llvm::StringRef message,
+                                  llvm::StringRef prompt) {
+  auto split_message_lines =
+      [&](llvm::StringRef message) -> std::vector<std::string> {
+    const char newline = '\n';
+    std::vector<std::string> return_value;
+
+    std::stringstream splitter((std::string(message)));
+    std::string split;
+
+    while (getline(splitter, split, newline)) {
+      return_value.push_back(split);
+    }
+
+    return return_value;
+  };
+
+  const auto message_lines = split_message_lines(message);
+  if (message_lines.size() != 3) {
+    return "";
+  }
+
+  llvm::StringRef caret_string = message_lines[2];
+
+  // Check for clang style diagnostic message.
+  if (caret_string.find("| ") != std::string::npos) {
+    const auto split = caret_string.split("| ");
+    caret_string = split.second;
+  }
+
+  const auto padding_size = prompt.size();
+  auto padded_caret_line = std::string(padding_size, ' ');
+
+  padded_caret_line += caret_string;
+  padded_caret_line + "\n";
+  return padded_caret_line;
+}
 
 void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
                                                 std::string &line) {
@@ -3127,8 +3165,25 @@ 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.GetErrorData();
-      PrintCommandOutput(io_handler, error, false);
+      // Print a caret just below developer's command, if there's a diagnostic.
+      std::vector<Diagnostic> diagnostics = result.GetDiagnostics();
+      if (diagnostics.size() >= 1) {
+        const auto prompt = GetDebugger().GetPrompt();
+        const auto first_message = diagnostics.front().GetMessage();
+        const auto caret_line = BuildCaretLine(first_message, prompt);
+
+        if (!caret_line.empty()) {
+          if (line.empty()) {
+            auto prompt_and_command = std::string(prompt);
+            prompt_and_command += m_repeat_command;
+            PrintCommandOutput(io_handler, prompt_and_command, false);
+          }
+          PrintCommandOutput(io_handler, caret_line, false);
+        }
+      }
+
+      llvm::StringRef error_string = result.GetErrorData();
+      PrintCommandOutput(io_handler, error_string, false);
     }
   }
 
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 0bc58124f3941f4..6f56d8e1a2d4c11 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -109,6 +109,8 @@ void CommandReturnObject::AppendError(llvm::StringRef in_string) {
 
 void CommandReturnObject::SetError(const Status &error,
                                    const char *fallback_error_cstr) {
+  m_diagnostics = error.GetDiagnostics();
+
   if (error.Fail())
     AppendError(error.AsCString(fallback_error_cstr));
 }
@@ -131,6 +133,10 @@ void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
 
 ReturnStatus CommandReturnObject::GetStatus() const { return m_status; }
 
+std::vector<Diagnostic> CommandReturnObject::GetDiagnostics() const {
+  return m_diagnostics;
+}
+
 bool CommandReturnObject::Succeeded() const {
   return m_status <= eReturnStatusSuccessContinuingResult;
 }
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 3bd00bb20da258c..952c0d1d40d9983 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -284,3 +284,11 @@ void llvm::format_provider<lldb_private::Status>::format(
   llvm::format_provider<llvm::StringRef>::format(error.AsCString(), OS,
                                                  Options);
 }
+
+void Status::AddDiagnostic(Diagnostic diagnostic) {
+  m_diagnostics.push_back(diagnostic);
+}
+
+std::vector<Diagnostic> Status::GetDiagnostics() const {
+  return m_diagnostics;
+}



More information about the lldb-commits mailing list