https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/110901
>From 262ff5a7a24403eaee1c5b9f95db118925494eb1 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <apra...@apple.com>
Date: Tue, 1 Oct 2024 17:39:10 -0700
Subject: [PATCH] Support inline diagnostics in CommandReturnObject
and implement them for dwim-print (a.k.a. `p`) as an example.
The next step will be to expose them as structured data in
SBCommandReturnObject.
---
.../lldb/Expression/DiagnosticManager.h | 33 ++-------------
.../lldb/Interpreter/CommandReturnObject.h | 15 ++++---
.../lldb/Utility/DiagnosticsRendering.h | 25 +++++++++++
lldb/include/lldb/Utility/Status.h | 31 ++++++++++++++
.../Commands/CommandObjectDWIMPrint.cpp | 2 +
.../Commands/CommandObjectExpression.cpp | 5 +--
.../source/Interpreter/CommandInterpreter.cpp | 15 +++++--
.../Interpreter/CommandReturnObject.cpp | 41 ++++++++++++++++++-
lldb/source/Utility/CMakeLists.txt | 1 +
.../DiagnosticsRendering.cpp} | 25 +++++------
.../Shell/Commands/command-dwim-print.test | 16 ++++++++
lldb/unittests/Interpreter/CMakeLists.txt | 1 -
lldb/unittests/Utility/CMakeLists.txt | 1 +
.../DiagnosticsRenderingTest.cpp} | 6 +--
14 files changed, 155 insertions(+), 62 deletions(-)
create mode 100644 lldb/include/lldb/Utility/DiagnosticsRendering.h
rename lldb/source/{Commands/DiagnosticRendering.h =>
Utility/DiagnosticsRendering.cpp} (83%)
create mode 100644 lldb/test/Shell/Commands/command-dwim-print.test
rename lldb/unittests/{Interpreter/TestCommandObjectExpression.cpp =>
Utility/DiagnosticsRenderingTest.cpp} (84%)
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h
b/lldb/include/lldb/Expression/DiagnosticManager.h
index b9a6421577781e..f90a8764e13962 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -23,35 +23,6 @@
namespace lldb_private {
-/// A compiler-independent representation of a Diagnostic. Expression
-/// evaluation failures often have more than one diagnostic that a UI
-/// layer might want to render differently, for example to colorize
-/// it.
-///
-/// Running example:
-/// (lldb) expr 1+foo
-/// error: <user expression 0>:1:3: use of undeclared identifier 'foo'
-/// 1+foo
-/// ^
-struct DiagnosticDetail {
- struct SourceLocation {
- FileSpec file;
- unsigned line = 0;
- uint16_t column = 0;
- uint16_t length = 0;
- bool hidden = false;
- bool in_user_input = false;
- };
- /// Contains {{}, 1, 3, 3, true} in the example above.
- std::optional<SourceLocation> source_location;
- /// Contains eSeverityError in the example above.
- lldb::Severity severity = lldb::eSeverityInfo;
- /// Contains "use of undeclared identifier 'x'" in the example above.
- std::string message;
- /// Contains the fully rendered error message.
- std::string rendered;
-};
-
/// An llvm::Error used to communicate diagnostics in Status. Multiple
/// diagnostics may be chained in an llvm::ErrorList.
class ExpressionError
@@ -65,7 +36,9 @@ class ExpressionError
ExpressionError(lldb::ExpressionResults result, std::string msg,
std::vector<DiagnosticDetail> details = {});
std::string message() const override;
- llvm::ArrayRef<DiagnosticDetail> GetDetails() const { return m_details; }
+ llvm::ArrayRef<DiagnosticDetail> GetDetails() const override {
+ return m_details;
+ }
std::error_code convertToErrorCode() const override;
void log(llvm::raw_ostream &OS) const override;
std::unique_ptr<CloneableError> Clone() const override;
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 8f6c9f123b7690..97b47148829ec8 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -29,6 +29,9 @@ class CommandReturnObject {
~CommandReturnObject() = default;
+ llvm::StringRef GetInlineDiagnosticsData(unsigned indent,
+ llvm::StringRef command);
+
llvm::StringRef GetOutputData() {
lldb::StreamSP
stream_sp(m_out_stream.GetStreamAtIndex(eStreamStringIndex));
if (stream_sp)
@@ -36,12 +39,7 @@ class CommandReturnObject {
return llvm::StringRef();
}
- llvm::StringRef GetErrorData() {
- lldb::StreamSP
stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex));
- if (stream_sp)
- return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
- return llvm::StringRef();
- }
+ llvm::StringRef GetErrorData();
Stream &GetOutputStream() {
// Make sure we at least have our normal string stream output stream
@@ -135,6 +133,8 @@ class CommandReturnObject {
void SetError(llvm::Error error);
+ void SetUserInput(llvm::StringRef user_input) { m_user_input = user_input; }
+
lldb::ReturnStatus GetStatus() const;
void SetStatus(lldb::ReturnStatus status);
@@ -160,6 +160,9 @@ class CommandReturnObject {
StreamTee m_out_stream;
StreamTee m_err_stream;
+ std::vector<DiagnosticDetail> m_diagnostics;
+ StreamString m_diag_stream;
+ std::string m_user_input;
lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
new file mode 100644
index 00000000000000..5488dbe278b5d0
--- /dev/null
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -0,0 +1,25 @@
+//===-- DiagnosticsRendering.h ----------------------------------*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_DIAGNOSTICSRENDERING_H
+#define LLDB_UTILITY_DIAGNOSTICSRENDERING_H
+
+#include "lldb/Utility/Status.h"
+#include "lldb/Utility/Stream.h"
+#include "llvm/Support/WithColor.h"
+
+namespace lldb_private {
+
+llvm::raw_ostream &PrintSeverity(Stream &stream, lldb::Severity severity);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional<uint16_t> offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef<DiagnosticDetail> details);
+} // namespace lldb_private
+#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index 3910c26d115a09..e2deb89e5226f1 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/Utility/FileSpec.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -26,6 +27,35 @@ class raw_ostream;
namespace lldb_private {
+/// A compiler-independent representation of a Diagnostic. Expression
+/// evaluation failures often have more than one diagnostic that a UI
+/// layer might want to render differently, for example to colorize
+/// it.
+///
+/// Running example:
+/// (lldb) expr 1+foo
+/// error: <user expression 0>:1:3: use of undeclared identifier 'foo'
+/// 1+foo
+/// ^
+struct DiagnosticDetail {
+ struct SourceLocation {
+ FileSpec file;
+ unsigned line = 0;
+ uint16_t column = 0;
+ uint16_t length = 0;
+ bool hidden = false;
+ bool in_user_input = false;
+ };
+ /// Contains {{}, 1, 3, 3, true} in the example above.
+ std::optional<SourceLocation> source_location;
+ /// Contains eSeverityError in the example above.
+ lldb::Severity severity = lldb::eSeverityInfo;
+ /// Contains "use of undeclared identifier 'foo'" in the example above.
+ std::string message;
+ /// Contains the fully rendered error message.
+ std::string rendered;
+};
+
const char *ExpressionResultAsCString(lldb::ExpressionResults result);
/// Going a bit against the spirit of llvm::Error,
@@ -86,6 +116,7 @@ class ExpressionErrorBase
ExpressionErrorBase(std::error_code ec, std::string msg = {})
: ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
+ virtual llvm::ArrayRef<DiagnosticDetail> GetDetails() const;
static char ID;
};
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 2e1d6e6e5af996..4b41d052d64a75 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -188,12 +188,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
ValueObjectSP valobj_sp;
std::string fixed_expression;
+ result.SetUserInput(expr);
ExpressionResults expr_result = target.EvaluateExpression(
expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
// Only mention Fix-Its if the expression evaluator applied them.
// Compiler errors refer to the final expression after applying Fix-It(s).
if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
+ result.SetUserInput(fixed_expression);
Stream &error_stream = result.GetErrorStream();
error_stream << " Evaluated this expression after applying
Fix-It(s):\n";
error_stream << " " << fixed_expression << "\n";
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp
b/lldb/source/Commands/CommandObjectExpression.cpp
index a72b409d21ed83..5762b75c9b962d 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -7,9 +7,7 @@
//===----------------------------------------------------------------------===//
#include "CommandObjectExpression.h"
-#include "DiagnosticRendering.h"
#include "lldb/Core/Debugger.h"
-#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Expression/REPL.h"
#include "lldb/Expression/UserExpression.h"
@@ -22,6 +20,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-enumerations.h"
@@ -490,7 +489,7 @@ bool
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
std::vector<DiagnosticDetail> details;
llvm::consumeError(llvm::handleErrors(
result_valobj_sp->GetError().ToError(),
- [&](ExpressionError &error) { details = error.GetDetails(); }));
+ [&](ExpressionErrorBase &error) { details = error.GetDetails();
}));
// Find the position of the expression in the command.
std::optional<uint16_t> expr_pos;
size_t nchar = m_original_command.find(expr);
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index d17aa6fec1f00e..ddded759a9cd7e 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2069,7 +2069,7 @@ bool CommandInterpreter::HandleCommand(const char
*command_line,
remainder.c_str());
// To test whether or not transcript should be saved, `transcript_item` is
- // used instead of `GetSaveTrasncript()`. This is because the latter will
+ // used instead of `GetSaveTranscript()`. This is because the latter will
// fail when the command is "settings set interpreter.save-transcript
true".
if (transcript_item) {
transcript_item->AddStringItem("commandName", cmd_obj->GetCommandName());
@@ -3180,15 +3180,24 @@ void
CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
if ((result.Succeeded() &&
io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) ||
io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors)) {
- // Display any STDOUT/STDERR _prior_ to emitting the command result text
GetProcessOutput();
+ // Display any inline diagnostics first.
+ if (!result.GetImmediateErrorStream() &&
+ GetDebugger().GetShowInlineDiagnostics() &&
+ line.find('\n') == std::string::npos) {
+ unsigned indent = GetDebugger().GetPrompt().size();
+ llvm::StringRef diags = result.GetInlineDiagnosticsData(indent, line);
+ PrintCommandOutput(io_handler, diags, true);
+ }
+
+ // Display any STDOUT/STDERR _prior_ to emitting the command result text.
if (!result.GetImmediateOutputStream()) {
llvm::StringRef output = result.GetOutputData();
PrintCommandOutput(io_handler, output, true);
}
- // Now emit the command error text from the command we just executed
+ // 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);
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp
b/lldb/source/Interpreter/CommandReturnObject.cpp
index d5da73c00a5209..70988d3de5ce0c 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -8,6 +8,7 @@
#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
@@ -112,8 +113,46 @@ void CommandReturnObject::SetError(Status error) {
}
void CommandReturnObject::SetError(llvm::Error error) {
- if (error)
+ // Retrieve any diagnostics.
+ error = llvm::handleErrors(std::move(error), [&](ExpressionErrorBase &error)
{
+ SetStatus(eReturnStatusFailed);
+ m_diagnostics = error.GetDetails();
+ });
+ if (error) {
AppendError(llvm::toString(std::move(error)));
+ }
+}
+
+llvm::StringRef
+CommandReturnObject::GetInlineDiagnosticsData(unsigned indent,
+ llvm::StringRef command) {
+ size_t nchar = command.find(m_user_input);
+ if (nchar == llvm::StringRef::npos)
+ return {};
+
+ indent += nchar;
+ RenderDiagnosticDetails(m_diag_stream, indent, true, m_diagnostics);
+ // Duplex the diagnostics to the secondary stream (but not inlined).
+ if (auto stream_sp = m_err_stream.GetStreamAtIndex(eStreamStringIndex))
+ RenderDiagnosticDetails(*stream_sp, std::nullopt, false, m_diagnostics);
+
+ // Clear them so GetErrorData() doesn't render them again.
+ m_diagnostics.clear();
+ return m_diag_stream.GetString();
+}
+
+llvm::StringRef CommandReturnObject::GetErrorData() {
+ // Diagnostics haven't been fetched; render them now (not inlined).
+ if (!m_diagnostics.empty()) {
+ RenderDiagnosticDetails(GetErrorStream(), std::nullopt, false,
+ m_diagnostics);
+ m_diagnostics.clear();
+ }
+
+ lldb::StreamSP stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex));
+ if (stream_sp)
+ return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
+ return llvm::StringRef();
}
// Similar to AppendError, but do not prepend 'Status: ' to message, and don't
diff --git a/lldb/source/Utility/CMakeLists.txt
b/lldb/source/Utility/CMakeLists.txt
index 397db0e8976023..6954a2508ffe1c 100644
--- a/lldb/source/Utility/CMakeLists.txt
+++ b/lldb/source/Utility/CMakeLists.txt
@@ -38,6 +38,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES
DataEncoder.cpp
DataExtractor.cpp
Diagnostics.cpp
+ DiagnosticsRendering.cpp
Environment.cpp
ErrorMessages.cpp
Event.cpp
diff --git a/lldb/source/Commands/DiagnosticRendering.h
b/lldb/source/Utility/DiagnosticsRendering.cpp
similarity index 83%
rename from lldb/source/Commands/DiagnosticRendering.h
rename to lldb/source/Utility/DiagnosticsRendering.cpp
index 5fdd090253a827..8bc3214d39a6f1 100644
--- a/lldb/source/Commands/DiagnosticRendering.h
+++ b/lldb/source/Utility/DiagnosticsRendering.cpp
@@ -1,4 +1,4 @@
-//===-- DiagnosticRendering.h -----------------------------------*- C++
-*-===//
+//===-- DiagnosticsRendering.cpp
------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,17 +6,14 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLDB_SOURCE_COMMANDS_DIAGNOSTICRENDERING_H
-#define LLDB_SOURCE_COMMANDS_DIAGNOSTICRENDERING_H
+#include "lldb/Utility/DiagnosticsRendering.h"
-#include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Utility/Stream.h"
-#include "llvm/Support/WithColor.h"
+using namespace lldb_private;
+using namespace lldb;
namespace lldb_private {
-static llvm::raw_ostream &PrintSeverity(Stream &stream,
- lldb::Severity severity) {
+llvm::raw_ostream &PrintSeverity(Stream &stream, lldb::Severity severity) {
llvm::HighlightColor color;
llvm::StringRef text;
switch (severity) {
@@ -36,12 +33,11 @@ static llvm::raw_ostream &PrintSeverity(Stream &stream,
return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable)
<< text;
}
-
-// Public for unittesting.
-static void RenderDiagnosticDetails(Stream &stream,
- std::optional<uint16_t> offset_in_command,
- bool show_inline,
- llvm::ArrayRef<DiagnosticDetail> details) {
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional<uint16_t> offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef<DiagnosticDetail> details) {
if (details.empty())
return;
@@ -130,4 +126,3 @@ static void RenderDiagnosticDetails(Stream &stream,
}
} // namespace lldb_private
-#endif
diff --git a/lldb/test/Shell/Commands/command-dwim-print.test
b/lldb/test/Shell/Commands/command-dwim-print.test
new file mode 100644
index 00000000000000..5603c5703f5dbe
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-dwim-print.test
@@ -0,0 +1,16 @@
+# RUN: echo quit | %lldb -o "dwim-print a" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK1
+# (lldb) dwim-print a
+# CHECK1:{{^ \^}}
+# CHECK1: {{^ ╰─ error: use of undeclared identifier 'a'}}
+# RUN: echo quit | %lldb -o "p a" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK2
+# (lldb) p a
+# CHECK2:{{^ \^}}
+# RUN: echo quit | %lldb -o "dwim-print -- a" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK3
+# (lldb) dwim-print -- a
+# CHECK3:{{^ \^}}
+# RUN: echo quit | %lldb -o "settings set show-inline-diagnostics false" \
+# RUN: -o "dwim-print a" 2>&1 | FileCheck %s --check-prefix=CHECK4
+# CHECK4: error: <user expression 0>:1:1: use of undeclared identifier
diff --git a/lldb/unittests/Interpreter/CMakeLists.txt
b/lldb/unittests/Interpreter/CMakeLists.txt
index f7d639f50f5bf3..d4ba5b3d58334a 100644
--- a/lldb/unittests/Interpreter/CMakeLists.txt
+++ b/lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,6 +1,5 @@
add_lldb_unittest(InterpreterTests
TestCommandPaths.cpp
- TestCommandObjectExpression.cpp
TestCompletion.cpp
TestOptionArgParser.cpp
TestOptions.cpp
diff --git a/lldb/unittests/Utility/CMakeLists.txt
b/lldb/unittests/Utility/CMakeLists.txt
index 40e0959fc01d14..3a73bdc3e9e1a2 100644
--- a/lldb/unittests/Utility/CMakeLists.txt
+++ b/lldb/unittests/Utility/CMakeLists.txt
@@ -10,6 +10,7 @@ add_lldb_unittest(UtilityTests
DataBufferTest.cpp
DataEncoderTest.cpp
DataExtractorTest.cpp
+ DiagnosticsRenderingTest.cpp
EnvironmentTest.cpp
EventTest.cpp
FileSpecListTest.cpp
diff --git a/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
similarity index 84%
rename from lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
rename to lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
index 9e3417b5428923..2bd80796b8074c 100644
--- a/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -1,4 +1,4 @@
-#include "../../source/Commands/DiagnosticRendering.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/Utility/StreamString.h"
#include "gtest/gtest.h"
@@ -7,13 +7,13 @@ using namespace lldb;
using llvm::StringRef;
namespace {
class ErrorDisplayTest : public ::testing::Test {};
-} // namespace
-static std::string Render(std::vector<DiagnosticDetail> details) {
+std::string Render(std::vector<DiagnosticDetail> details) {
StreamString stream;
RenderDiagnosticDetails(stream, 0, true, details);
return stream.GetData();
}
+} // namespace
TEST_F(ErrorDisplayTest, RenderStatus) {
DiagnosticDetail::SourceLocation inline_loc;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
