https://github.com/PortalPete updated https://github.com/llvm/llvm-project/pull/72150
>From 2e886082f69d85ea719339aa4917c744492988c4 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 | 15 +-- .../lldb/Interpreter/CommandInterpreter.h | 3 + .../lldb/Interpreter/CommandReturnObject.h | 3 + lldb/include/lldb/Utility/Status.h | 103 ++++++++++++++++++ lldb/include/lldb/lldb-private-enumerations.h | 14 +++ lldb/source/Expression/UserExpression.cpp | 19 +++- .../source/Interpreter/CommandInterpreter.cpp | 33 ++++++ .../Interpreter/CommandReturnObject.cpp | 19 +++- lldb/source/Utility/Status.cpp | 8 ++ 9 files changed, 198 insertions(+), 19 deletions(-) diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 06bf1d115f1541..b4c888066943df 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/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 { diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 747188a15312fa..03b9cc5c8ee484 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -688,6 +688,9 @@ class CommandInterpreter : public Broadcaster, StringList &commands_help, const CommandObject::CommandMap &command_map); + void PrintCaretIndicator(StatusDetail detail, IOHandler &io_handler, + std::string &command_line); + // An interruptible wrapper around the stream output void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str, bool is_stdout); diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index 8c4dcb54d708f0..4ac89a6344db5b 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -139,6 +139,8 @@ class CommandReturnObject { void SetStatus(lldb::ReturnStatus status); + std::vector<StatusDetail> GetStatusDetails() const; + bool Succeeded() const; bool HasResult() const; @@ -162,6 +164,7 @@ class CommandReturnObject { StreamTee m_err_stream; lldb::ReturnStatus m_status = lldb::eReturnStatusStarted; + std::vector<StatusDetail> m_status_details; 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 fa5768141fa45d..1c4bb8caee97c9 100644 --- a/lldb/include/lldb/Utility/Status.h +++ b/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" @@ -16,6 +17,7 @@ #include "llvm/Support/FormatVariadic.h" #include <cstdarg> #include <cstdint> +#include <sstream> #include <string> #include <system_error> #include <type_traits> @@ -25,6 +27,101 @@ class raw_ostream; } namespace lldb_private { +struct StatusDetail { +private: + std::vector<std::string> m_message_lines; + DiagnosticSeverity m_message_type; + DiagnosticOrigin m_message_origin; + + // Lazy, cmputed properties + mutable std::optional<std::string> m_message; + mutable std::optional<std::string> m_caret_string; + + static std::string StringForSeverity(DiagnosticSeverity severity) { + switch (severity) { + case lldb_private::eDiagnosticSeverityError: + return std::string("error: "); + case lldb_private::eDiagnosticSeverityWarning: + return std::string("warning: "); + case lldb_private::eDiagnosticSeverityRemark: + return std::string("note: "); + } + } + + std::vector<std::string> + GetMessageLinesFromDiagnostic(Diagnostic *diagnostic) { + const char newline = '\n'; + std::vector<std::string> message_lines; + + std::stringstream splitter(std::string(diagnostic->GetMessage())); + std::string split; + + while (getline(splitter, split, newline)) { + message_lines.push_back(split); + } + + return message_lines; + } + +public: + StatusDetail() = default; + StatusDetail(const std::unique_ptr<Diagnostic> &diagnostic) + : m_message_lines(GetMessageLinesFromDiagnostic(diagnostic.get())), + m_message_type(diagnostic->GetSeverity()), + m_message_origin(diagnostic->getKind()) {} + + StatusDetail(const StatusDetail &other) + : m_message_lines(other.m_message_lines), + m_message_type(other.m_message_type), + m_message_origin(other.m_message_origin) {} + + StatusDetail &operator=(const StatusDetail &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::vector<std::string> GetMessageLines() const { return m_message_lines; } + + std::string GetMessage() const { + if (m_message) + return m_message.value(); + + std::string severity = StringForSeverity(m_message_type); + std::string message = severity; + for (auto line : m_message_lines) { + size_t start = line.find(severity); + if (start != std::string::npos) + line.erase(start, severity.length()); + + message += line + "\n"; + } + message += "\n"; + m_message = message; + return message; + } + + std::optional<std::string> GetCaretString() const { + if (m_caret_string) + return m_caret_string; + + if (m_message_lines.size() != 3) { + return {}; + } + + llvm::StringRef caret_string = m_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; + } + + m_caret_string = caret_string; + return m_caret_string; + } +}; /// \class Status Status.h "lldb/Utility/Status.h" An error handling class. /// @@ -180,12 +277,18 @@ class Status { /// success (non-erro), \b false otherwise. bool Success() const; + void AddDetail(StatusDetail detail); + + std::vector<StatusDetail> 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<StatusDetail> m_status_details; + private: explicit Status(const llvm::formatv_object_base &payload) { SetErrorToGenericError(); diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 7f98220f9f1643..1242b8b8109ab4 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -231,6 +231,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: diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp index c181712a2f0b24..aca7ac86c958f9 100644 --- a/lldb/source/Expression/UserExpression.cpp +++ b/lldb/source/Expression/UserExpression.cpp @@ -278,6 +278,21 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx, user_expression_sp->Parse(diagnostic_manager, exe_ctx, execution_policy, keep_expression_in_memory, generate_debug_info); + // Copy each diagnostic from the `Staus &error` instance's details list. + { + const DiagnosticList &diagnostics = diagnostic_manager.Diagnostics(); + + if (diagnostics.size() >= 1) { + std::vector<StatusDetail> status_details = error.GetDetails(); + status_details.clear(); + + for (auto &diagnostic : diagnostics) { + StatusDetail detail(diagnostic); + error.AddDetail(detail); + } + } + } + // Calculate the fixed expression always, since we need it for errors. std::string tmp_fixed_expression; if (fixed_expression == nullptr) @@ -327,9 +342,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx, std::string msg; { llvm::raw_string_ostream os(msg); - if (!diagnostic_manager.Diagnostics().empty()) - os << diagnostic_manager.GetString(); - else + if (diagnostic_manager.Diagnostics().empty()) os << "expression failed to parse (no further compiler diagnostics)"; if (target->GetEnableNotifyAboutFixIts() && fixed_expression && !fixed_expression->empty()) diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index e1275ce711fc17..ba5d85f0c86160 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -3031,6 +3031,33 @@ bool CommandInterpreter::WasInterrupted() const { lldbassert(!was_interrupted || m_iohandler_nesting_level > 0); return was_interrupted; } +void CommandInterpreter::PrintCaretIndicator(StatusDetail detail, + IOHandler &io_handler, + std::string &command_line) { + // Check whether the first detail has an indicator caret (^) within it. + const auto message_lines = detail.GetMessageLines(); + const auto caret_string_opt = detail.GetCaretString(); + if (message_lines.size() >= 3 && caret_string_opt) { + const auto prompt = GetDebugger().GetPrompt(); + // Check whether the developer just hit [Return] to repeat a command. + if (command_line.empty()) { + // Build a recreation of the prompt and the last command. + auto prompt_and_command = std::string(prompt); + prompt_and_command += m_repeat_command; + + // Print the last command the developer entered for the caret line. + PrintCommandOutput(io_handler, prompt_and_command, false); + } + + // Print the line with the indicator caret (^) below the command. + auto padding_size = prompt.size(); + padding_size += command_line.size() - message_lines[1].size(); + const std::string caret_line = + std::string(padding_size, ' ') + caret_string_opt.value() + "\n"; + + PrintCommandOutput(io_handler, caret_line, false); + } +} void CommandInterpreter::PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str, @@ -3127,6 +3154,12 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, // Now emit the command error text from the command we just executed if (!result.GetImmediateErrorStream()) { + + // Check whether there's any status details. + std::vector<StatusDetail> details = result.GetStatusDetails(); + if (!details.empty()) + PrintCaretIndicator(details.front(), io_handler, line); + llvm::StringRef error = result.GetErrorData(); PrintCommandOutput(io_handler, error, false); } diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 0bc58124f3941f..4d617839789efb 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -109,8 +109,19 @@ 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_status_details = error.GetDetails(); + + if (error.Fail()) { + if (m_status_details.size() > 0) { + std::string messages; + + for (StatusDetail detail : m_status_details) + messages += detail.GetMessage(); + + AppendError(messages); + } else + AppendError(error.AsCString(fallback_error_cstr)); + } } void CommandReturnObject::SetError(llvm::Error error) { @@ -131,6 +142,10 @@ void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; } ReturnStatus CommandReturnObject::GetStatus() const { return m_status; } +std::vector<StatusDetail> CommandReturnObject::GetStatusDetails() const { + return m_status_details; +} + bool CommandReturnObject::Succeeded() const { return m_status <= eReturnStatusSuccessContinuingResult; } diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp index 3bd00bb20da258..eb3b692cef4c7d 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::AddDetail(StatusDetail detail) { + m_status_details.push_back(detail); +} + +std::vector<StatusDetail> Status::GetDetails() const { + return m_status_details; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits