[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
Wed Dec 13 16:00:58 PST 2023
https://github.com/PortalPete updated https://github.com/llvm/llvm-project/pull/72150
>From 0e969a1d7b198d586108a4177ecc1fc32b372e24 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 | 15 +--
.../lldb/Interpreter/CommandInterpreter.h | 2 +
.../lldb/Interpreter/CommandReturnObject.h | 4 +
lldb/include/lldb/Utility/Status.h | 105 ++++++++++++++++++
lldb/include/lldb/lldb-private-enumerations.h | 14 +++
lldb/source/Expression/UserExpression.cpp | 19 +++-
.../source/Interpreter/CommandInterpreter.cpp | 31 ++++++
.../Interpreter/CommandReturnObject.cpp | 20 +++-
lldb/source/Utility/Status.cpp | 8 ++
9 files changed, 199 insertions(+), 19 deletions(-)
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index 06bf1d115f1541..904cee4403abaa 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -11,6 +11,7 @@
#include "lldb/lldb-defines.h"
#include "lldb/lldb-types.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.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..5062c05c8e98c1 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -688,6 +688,8 @@ 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..b063d6723d1ea8 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<StatusDetail> GetStatusDetails() 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<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..90e60e379a3620 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,103 @@ 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 +279,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..90098016806181 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3031,6 +3031,31 @@ 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 +3152,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..bf01b8f8f6f80f 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -109,8 +109,20 @@ 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 +143,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;
+}
More information about the lldb-commits
mailing list