[Lldb-commits] [lldb] [lldb] DRAFT - Print multiple messages with line art that goes from each indicator (PR #80938)
Pete Lawrence via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 6 21:20:47 PST 2024
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 <plawrence at 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 <plawrence at 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");
More information about the lldb-commits
mailing list