[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
Mon Nov 13 10:58:26 PST 2023
https://github.com/PortalPete created https://github.com/llvm/llvm-project/pull/72150
This patch serves 2 goals:
- Remove a second "error: " substring in error messages from debugging Swift.
- Print caret (^) line with optional leading/trailing tildes (~) below the line the developer last entered.
Here' an example from a Swift app:
```
(lldb) p abc
error: <EXPR>:3:1: error: cannot find 'abc' in scope
abc
^~~
```
And here's what that same error message is after the patch
```
(lldb) p abc
^~~
error: <EXPR>:3:1: cannot find 'abc' in scope
abc
^~~
```
To make this work, the patch passes the diagnostics up the call stack to `CommandInterpreter::IOHandlerInputComplete(...)` because the deepest call frame that knows what the developer entered at the prompt. This is important for commands like DWIM print (and its aliases 'p' and 'po') where the length of the actual command they typed in gets us the correct spacing to pad before the line with the caret.
To get the diagnostics up to that level, I added a vector of `Diagnostic` instances into `Status` and `CommandReturnObject`. I originally prototyped this by hoisting a `DiagnosticManager` manager instance but it's a bit heavy handed for what we need to accomplish the goal.
>From 0b25a5adb9579f162cc64a3e8c2aa8ebb94bc74c 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/Interpreter/CommandReturnObject.h | 6 ++
lldb/include/lldb/Utility/Status.h | 2 +
lldb/source/Expression/UserExpression.cpp | 14 +++++
.../source/Interpreter/CommandInterpreter.cpp | 59 ++++++++++++++++++-
.../Interpreter/CommandReturnObject.cpp | 10 ++++
5 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 8c4dcb54d708f08..41afcd654928287 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,10 @@ class CommandReturnObject {
void SetStatus(lldb::ReturnStatus status);
+ std::vector<Diagnostic> GetDiagnostics() const;
+
+ void SetDiagnostics(std::vector<Diagnostic> diagnostics);
+
bool Succeeded() const;
bool HasResult() const;
@@ -162,6 +167,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 ac410552438e0c6..7df3436b6c4b8ed 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"
@@ -188,6 +189,7 @@ class Status {
/// \b true if this object contains an value that describes
/// success (non-erro), \b false otherwise.
bool Success() const;
+ mutable std::vector<Diagnostic> m_diagnostics;
protected:
/// Member variables
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index c181712a2f0b243..98da9de91a2b8b6 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.m_diagnostics;
+ 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..1ea9d948a54615d 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.m_diagnostics;
+
if (error.Fail())
AppendError(error.AsCString(fallback_error_cstr));
}
@@ -131,6 +133,14 @@ void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
ReturnStatus CommandReturnObject::GetStatus() const { return m_status; }
+std::vector<Diagnostic> CommandReturnObject::GetDiagnostics() const {
+ return m_diagnostics;
+}
+
+void CommandReturnObject::SetDiagnostics(std::vector<Diagnostic> diagnostics) {
+ m_diagnostics = diagnostics;
+}
+
bool CommandReturnObject::Succeeded() const {
return m_status <= eReturnStatusSuccessContinuingResult;
}
More information about the lldb-commits
mailing list