https://github.com/PortalPete created https://github.com/llvm/llvm-project/pull/80938
This changes the way `dwim` and `expr` commands display 1 or more errors, warnings, and remarks with line art in a style that inspires the name "pan flute". ### Before The way it looks today. <img width="1368" alt="image" src="https://github.com/llvm/llvm-project/assets/34425917/4252435f-29ea-486b-ba7d-d2b0d3d370e7";> ### After In the pan-flute style. <img width="1368" alt="image" src="https://github.com/llvm/llvm-project/assets/34425917/eabded99-e72b-4a34-8bd6-59e0d7ad9e41";> **Note** This PR requires changes from PR 80936, which store information from each diagnostic in a vector of `Status::Detail` instances within a `Status` object. The changes in the 3 files prints each detail's message separately, colorizes each detail's text based on the severity, and forms lines in output that connect the message to the position indicator (`^`) that shows the part of the expression the message applies to. The differences beyond that PR are in these 3 files: - lldb/include/lldb/Interpreter/CommandReturnObject.h - lldb/source/Interpreter/CommandReturnObject.cpp - lldb/source/Interpreter/CommandInterpreter.cpp Changes for this "pan flute" implementation: 1. Add new `CommandReturnObject::DetailStringForPromptCommand(...)` method that prints the messages and line art. - The method uses regular expressions to parse out the message - It works for messages from both the `clang` and `swift` compilers. - The method prints the messages in reverse order to keep the lines from crossing each other. - Added static function `llvm::raw_ostream &remark(Stream &strm)` - Consistent with its peers, `error(Stream &strm)` and `warning(Stream &strm) ` - Remarks use a different color than errors warnings. - There's an opportunity to add a `note` version of the function, but there's not need for it at this time. 2. Modify the `CommandInterpreter::IOHandlerInputComplete` method - It calls the new method under the right circumstances. - It reprints the previous last typed command if the developer simply hit [return] to repeat the last command - That way, the line hard always has something to "point" to for the messages. >From 575e4922c9bfb060c7bb81285fed3b07fe62c0fe Mon Sep 17 00:00:00 2001 From: Pete Lawrence <plawre...@apple.com> Date: Wed, 20 Dec 2023 15:52:05 -1000 Subject: [PATCH 1/2] [lldb] Copy diagnostic information into a Status with its new StatusDetail vector. Changes: * + Type `struct Status::Detail` type in `Status.cpp` * + Member `std::vector<Status::Detail> m_status_details` in `class Status` * + Related methods * + Member `std::vector<Status::Detail> m_status_details` in `class CommandReturnObject` * Types moved from `DiagnosticManager.h` -> `lldb-private-enumerations.h` * `enum DiagnosticOrigin` * `enum DiagnosticSeverity` * The `UserExpression::Evaluate` method: * Creates `Status::Detail` for `Diagnostic` in a `DiagnosticManager` instance. * And adds it to its `Status &error` parameter. * No longer appends the diagnostic manager's string (of its diagnostics) to its `&error` parameter. * The `CommandReturnObject::SetError` method: * Saves a copy of an error (Status). * Appends each status detail (diagnostic) into its own separate error messages. * This lets `CommandReturnObject` individually colorize each "error:", "warning:", or "note:". * Before, only the first entry got the colorization because the remaining entries were just concatenated onto the first entry. --- .../lldb/Expression/DiagnosticManager.h | 15 +-- .../lldb/Interpreter/CommandReturnObject.h | 1 + lldb/include/lldb/Utility/Status.h | 46 +++++++- lldb/include/lldb/lldb-private-enumerations.h | 14 +++ lldb/source/Breakpoint/BreakpointLocation.cpp | 27 ++--- lldb/source/Expression/UserExpression.cpp | 24 ++-- lldb/source/Expression/UtilityFunction.cpp | 20 ++-- .../Interpreter/CommandReturnObject.cpp | 11 +- .../Clang/ClangExpressionParser.cpp | 6 +- .../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 11 +- .../Platform/Windows/PlatformWindows.cpp | 10 +- lldb/source/Utility/Status.cpp | 103 +++++++++++++++++- 12 files changed, 221 insertions(+), 67 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/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index 8c4dcb54d708f0..3efbde9b314746 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/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; diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h index fa5768141fa45d..17e3257e435a40 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" @@ -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,12 +181,20 @@ 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(); @@ -193,6 +202,41 @@ class Status { } }; +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 { diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 5f1597200a83e7..004d79112e1ecb 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/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: diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp index 98de059c2e2967..9a8d7219c06041 100644 --- a/lldb/source/Breakpoint/BreakpointLocation.cpp +++ b/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; diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp index c181712a2f0b24..03d3c93d5b79af 100644 --- a/lldb/source/Expression/UserExpression.cpp +++ b/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(); diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp index d7a3c9d41d0484..9efebec05f76c6 100644 --- a/lldb/source/Expression/UtilityFunction.cpp +++ b/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; } diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 0bc58124f3941f..6a640cd365f455 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/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) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 574d661e2a215e..6f6180ae0952e9 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/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; } } diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index b4f1b76c39dbeb..f285d7c0abf97c 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/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; } diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index 88d543289a8470..aded7fc7556871 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/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; } diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp index 3bd00bb20da258..9575c1d6146350 100644 --- a/lldb/source/Utility/Status.cpp +++ b/lldb/source/Utility/Status.cpp @@ -18,6 +18,7 @@ #include <cerrno> #include <cstdarg> +#include <sstream> #include <string> #include <system_error> @@ -160,6 +161,18 @@ const char *Status::AsCString(const char *default_error_str) const { else return nullptr; // User wanted a nullptr string back... } + + if (!m_status_details.empty()) { + if (!m_string_with_details.size()) + return m_string_with_details.c_str(); + + m_string_with_details = m_string; + for (Status::Detail detail : m_status_details) + m_string_with_details += detail.GetMessage(); + + return m_string_with_details.c_str(); + } + return m_string.c_str(); } @@ -168,6 +181,8 @@ void Status::Clear() { m_code = 0; m_type = eErrorTypeInvalid; m_string.clear(); + m_string_with_details.clear(); + m_status_details.clear(); } // Access the error value. @@ -181,10 +196,14 @@ ErrorType Status::GetType() const { return m_type; } bool Status::Fail() const { return m_code != 0; } void Status::SetExpressionError(lldb::ExpressionResults result, - const char *mssg) { + const char *message) { m_code = result; m_type = eErrorTypeExpression; - m_string = mssg; + + if (message) + m_string = message; + else + m_string.clear(); } int Status::SetExpressionErrorWithFormat(lldb::ExpressionResults result, @@ -274,6 +293,18 @@ int Status::SetErrorStringWithVarArg(const char *format, va_list args) { return 0; } +void Status::SetErrorDetails(DiagnosticManager &diagnostic_manager) { + m_status_details.clear(); + m_string_with_details.clear(); + + const DiagnosticList &diagnostics = diagnostic_manager.Diagnostics(); + if (diagnostics.empty()) + return; + + for (auto &diagnostic : diagnostics) + m_status_details.push_back(Status::Detail(diagnostic)); +} + // Returns true if the error code in this object is considered a successful // return value. bool Status::Success() const { return m_code == 0; } @@ -284,3 +315,71 @@ void llvm::format_provider<lldb_private::Status>::format( llvm::format_provider<llvm::StringRef>::format(error.AsCString(), OS, Options); } + +void Status::AddDetail(Status::Detail detail) { + m_status_details.push_back(detail); + m_string_with_details.clear(); +} + +std::vector<Status::Detail> Status::GetDetails() const { + return m_status_details; +} + +std::string Status::Detail::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> +Status::Detail::GetMessageLinesFromDiagnostic(Diagnostic *diagnostic) { + const char newline = '\n'; + std::vector<std::string> message_lines; + + std::string severity = StringForSeverity(m_message_type); + std::string message = severity; + + std::stringstream splitter(std::string(diagnostic->GetMessage())); + std::string split_line; + + while (getline(splitter, split_line, newline)) { + size_t position = split_line.find(severity); + if (position != std::string::npos) + split_line.erase(position, severity.length()); + + // Check for clang-style diagnostic message. + std::string separator(" | "); + position = split_line.find(separator); + if (split_line.find(separator) != std::string::npos) { + auto end = split_line.begin() + position + separator.size(); + split_line.erase(split_line.begin(), end); + } + + message_lines.push_back(split_line); + } + + return message_lines; +} + +std::vector<std::string> Status::Detail::GetMessageLines() const { + return m_message_lines; +} + +std::string Status::Detail::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) + message += line + "\n"; + + message += "\n"; + m_message = message; + return message; +} >From 077a67b524b3216c34a770813c1705eb2529aed5 Mon Sep 17 00:00:00 2001 From: Pete Lawrence <plawre...@apple.com> Date: Fri, 5 Jan 2024 20:02:07 -1000 Subject: [PATCH 2/2] [lldb] Print errors/warnings/remarks with line art to the various locations in an expression. --- .../lldb/Interpreter/CommandReturnObject.h | 3 + .../source/Interpreter/CommandInterpreter.cpp | 22 ++- .../Interpreter/CommandReturnObject.cpp | 146 ++++++++++++++++++ 3 files changed, 169 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index 3efbde9b314746..d63f018972e75e 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -133,6 +133,9 @@ class CommandReturnObject { void SetError(const Status &error, const char *fallback_error_cstr = nullptr); + std::string DetailStringForPromptCommand(size_t prompt_size, + llvm::StringRef input); + void SetError(llvm::Error error); lldb::ReturnStatus GetStatus() const; diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 00651df48b6224..0d47547c2fd2ae 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -3131,8 +3131,26 @@ 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); + auto prompt_size = GetDebugger().GetPrompt().size(); + auto command = line.empty() ? m_repeat_command : line; + auto detail_string = + result.DetailStringForPromptCommand(prompt_size, command); + if (detail_string.empty()) { + llvm::StringRef error = result.GetErrorData(); + PrintCommandOutput(io_handler, error, false); + } else { + // Check whether the developer just hit [Return] to repeat a command. + if (line.empty()) { + // Build a recreation of the prompt and the last command. + std::string prompt_and_command(GetDebugger().GetPrompt()); + prompt_and_command += m_repeat_command; + + // Print the last command the developer entered for the caret line. + PrintCommandOutput(io_handler, prompt_and_command, false); + } + + PrintCommandOutput(io_handler, detail_string, false); + } } } diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 6a640cd365f455..538ee13dc0fc0d 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -11,6 +11,8 @@ #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" +#include <regex> + using namespace lldb; using namespace lldb_private; @@ -26,6 +28,12 @@ static llvm::raw_ostream &warning(Stream &strm) { << "warning: "; } +static llvm::raw_ostream &remark(Stream &strm) { + return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Remark, + llvm::ColorMode::Enable) + << "remark: "; +} + static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) { bool add_newline = false; if (!s.empty()) { @@ -128,6 +136,144 @@ void CommandReturnObject::SetError(llvm::Error error) { // Similar to AppendError, but do not prepend 'Status: ' to message, and don't // append "\n" to the end of it. +std::string +CommandReturnObject::DetailStringForPromptCommand(size_t prompt_size, + llvm::StringRef input) { + + if (input.empty() || m_error_status.GetDetails().empty()) + return ""; + + StreamString stream(true); + + struct ExpressionNote { + std::string message; + DiagnosticSeverity type; + size_t column; + }; + std::vector<ExpressionNote> notes; + const size_t not_found = std::string::npos; + + // Start off with a sentinel value; + size_t expression_position = not_found; + + auto note_builder = + [&](Status::Detail detail) -> std::optional<ExpressionNote> { + std::vector<std::string> detail_lines = detail.GetMessageLines(); + + // This function only knows how to parse messages with 3 lines. + if (detail_lines.size() != 3) + return {}; + + const DiagnosticSeverity type = detail.GetType(); + const std::string message = detail_lines[0]; + const std::string expression = detail_lines[1]; + const std::string indicator_line = detail_lines[2]; + + // Set the position if this is the first time. + if (expression_position == not_found) { + expression_position = input.find(expression); + + // Exit early if the expression isn't in the input string. + if (expression_position == not_found) + return {}; + } + // Ensure this note's expression has the same position as the note before. + else if (input.find(expression) != expression_position) + return {}; + + // Ensure the 3rd line has an indicator (^) in it. + if (not_found == indicator_line.find("^")) + return {}; + + // The regular expression pattern that isolates the indicator column. + // - ^ matches the start of a line + // - (.*?) matches any character before the angle left angle bracket (<) + // - <(.*?)> matches any characters inside the left angle brackets (<...>) + // - :([[:digit:]]+): matches 1+ digits between the 1st and 2nd colons (:) + // - ([[:digit:]]+): matches 1+ digits between the 2nd and 3rd colons (:) + const std::regex rx{"^(.*?)<(.*?)>:([[:digit:]]+):([[:digit:]]+):"}; + std::smatch match; + + // Exit if the format doesn't match. + if (!std::regex_search(message, match, rx)) + return {}; + + // std::string preamble(match[1]); + // std::string message_type(match[2]); + // std::string line_string(match[3]); + std::string column_string(match[4]); + + unsigned long long column; + // Convert the column number string into an integer value. + if (llvm::StringRef(column_string).consumeInteger(10, column)) + return {}; + + // Column values start at 1. + if (column <= 0) + return {}; + + ExpressionNote note; + note.message = message; + note.type = type; + note.column = column; + return note; + }; + + // Build a list of notes from the details. + for (Status::Detail &detail : m_error_status.GetDetails()) { + if (auto note_optional = note_builder(detail)) + notes.push_back(note_optional.value()); + else + return ""; + } + + // Print a line with caret indicator(s) below the lldb prompt + command. + const size_t padding = prompt_size + expression_position; + stream << std::string(padding, ' '); + + size_t offset = 1; + for (ExpressionNote note : notes) { + stream << std::string(note.column - offset, ' ') << '^'; + offset = note.column + 1; + } + stream << '\n'; + + // Work through each note in reverse order using the vector/stack. + while (!notes.empty()) { + // Get the information to print this note and remove it from the stack. + ExpressionNote this_note = notes.back(); + notes.pop_back(); + + // Print all the lines for all the other messages first. + stream << std::string(padding, ' '); + size_t offset = 1; + for (ExpressionNote remaining_note : notes) { + stream << std::string(remaining_note.column - offset, ' ') << "│"; + offset = remaining_note.column + 1; + } + + // Print the line connecting the ^ with the error message. + stream << std::string(this_note.column - offset, ' ') << "╰─ "; + + // Print a colorized string based on the message's severity type. + switch (this_note.type) { + case eDiagnosticSeverityError: + error(stream); + break; + case eDiagnosticSeverityWarning: + warning(stream); + break; + case eDiagnosticSeverityRemark: + remark(stream); + break; + } + + // Finally, print the message and start a new line. + stream << this_note.message << '\n'; + } + return stream.GetData(); +} + void CommandReturnObject::AppendRawError(llvm::StringRef in_string) { SetStatus(eReturnStatusFailed); assert(!in_string.empty() && "Expected a non-empty error message"); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
