https://github.com/PortalPete updated https://github.com/llvm/llvm-project/pull/72150
>From af3e1a6dce00477afd1418cc41fde6a2f8c17258 Mon Sep 17 00:00:00 2001 From: Pete Lawrence <plawre...@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 | 71 +++++++++++++++++++ lldb/source/Expression/UserExpression.cpp | 14 ++++ .../source/Interpreter/CommandInterpreter.cpp | 59 ++++++++++++++- .../Interpreter/CommandReturnObject.cpp | 6 ++ lldb/source/Utility/Status.cpp | 6 ++ 7 files changed, 159 insertions(+), 68 deletions(-) diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index df9ba3b245f51e8..79fa6bf24da1a1e 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -12,6 +12,7 @@ #include "lldb/lldb-defines.h" #include "lldb/lldb-types.h" +#include "lldb/Utility/Status.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" @@ -20,74 +21,8 @@ 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..c044146c85af7b1 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,18 @@ 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..0fc81f296214d89 100644 --- a/lldb/source/Utility/Status.cpp +++ b/lldb/source/Utility/Status.cpp @@ -284,3 +284,9 @@ 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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits