Support inline diagnostics in CommandReturnObject #110901
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) Changesand implement them for dwim-print (a.k.a. The next step will be to expose them as structured data in SBCommandReturnObject. Full diff: https://github.com/llvm/llvm-project/pull/110901.diff 14 Files Affected:
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..170f86f5729b41 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,44 @@ 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);
+
+ 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;
|
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
from
0b51cd8 to
262ff5a
Compare
|
Side note: I also think that it should be possible to change CommandObjectExpression to just put the error into CommendReturnObject instead of printing it to the error stream directly. But that's for a future patch. |
jimingham
reviewed
Oct 3, 2024
jimingham
reviewed
Oct 3, 2024
jimingham
reviewed
Oct 3, 2024
jimingham
reviewed
Oct 3, 2024
jimingham
reviewed
Oct 3, 2024
jimingham
reviewed
Oct 3, 2024
| } | ||
|
|
||
| llvm::StringRef | ||
| CommandReturnObject::GetInlineDiagnosticsData(unsigned indent, |
There was a problem hiding this comment.
This API seems wrong to me. You have the CommandReturnObject being made up for some command that got executed, so I should know what that command was. Then I have another API where I can pass in (if I feel like it) an entirely unrelated string as the "command". Then I have some m_user_input which is some subset of what the user actually input for the command.
Maybe I'm missing something, but this is confusing to me.
There was a problem hiding this comment.
Agreed. I made it slightly less weird by moving the indent computation into CommandInterpreter, but I still can't say I like it.
|
I find this API confusing, and it seems very focused on pointing out errors in commands that have some part of their text that gets submitted to a compiler. The other obvious candidate for this feature is a general command parser implementation that handles: (Apparently I can't get my ascii-art to work, but you get the idea...) |
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
2 times, most recently
from
5eff438 to
f59f186
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
2 times, most recently
from
bfb74e7 to
97b3812
Compare
|
@jimingham I changed the API to just record the offset of the expression in the command instead of the expression itself, which should make this all look a bit more logical. |
jimingham
reviewed
Oct 8, 2024
| /// Contains "use of undeclared identifier 'foo'" in the example above. | ||
| std::string message; | ||
| /// Contains the fully rendered error message, without "error: ", | ||
| /// but including the source context. |
There was a problem hiding this comment.
What does this contain for "inline display"?
There was a problem hiding this comment.
@jimingham I don't understand the question. Concretely, in the example rendered is
<user expression 0>:1:3: use of undeclared identifier 'foo'
/// 1 + foo
/// ^~~
this is what a client can use if they choose to not show diagnostics inline.
There was a problem hiding this comment.
I was confused because it didn't seem like there was a way to get the inline rendering if you wanted to use that, for instance if you were writing your own driver. Presumably there's no way to do that? That set me to wondering whether is_user_input = true would affect what you see in rendered?
You are using "the fully rendered error message" to mean "the one not rendered for inline display". That wasn't clear to me.
BTW, I don't actually think it's necessary to have a way to fetch the inline rendered form (properly offset for the prompt string, etc.) The possibility set me wondering, however.
There was a problem hiding this comment.
There is no "fetching of the inline rendered form". Either you use the prerendered non-inline form or you have to render the message yourself using all the details in the struct. The point is to allow different drivers to render errors in a way that makes sense for them.
There was a problem hiding this comment.
You have a bool saying whether this can be used in inline displays, which leads to the question "can I get you to make that for me here". That the answer to that is no is fine, butsaying that explicitly would have reduced confusion. No biggie...
jimingham
reviewed
Oct 8, 2024
jimingham
reviewed
Oct 8, 2024
|
|
||
| const char *ExpressionResultAsCString(lldb::ExpressionResults result); | ||
|
|
||
| llvm::raw_ostream &PrintSeverity(Stream &stream, lldb::Severity severity); |
There was a problem hiding this comment.
This one should also live wherever the ExpressionResults renderer is.
|
You didn't introduce the situation where we have two separate "ExpressionResults -> String" API's in different places, but you did move one, so maybe it's time to put them together? I don't have a strong opinion where they should go, except together if possible... I'll still be curious to see how this works for any non-expression based command errors. But ultimately if all it works for is expressions, it will still be doing a good thing, so... |
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
4 times, most recently
from
0a78922 to
49b51ba
Compare
jimingham
reviewed
Oct 10, 2024
jimingham
reviewed
Oct 10, 2024
|
Except for the test request, this LGTM. |
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
5 times, most recently
from
ab8957d to
ae9dce9
Compare
and implement them for dwim-print (a.k.a. `p`) and the command option parser (in one place thus far) as an example. The next step will be to expose them as structured data in SBCommandReturnObject.
adrian-prantl
force-pushed
the
dwim-print-diagnostics
branch
from
ae9dce9 to
b6caeb8
Compare
adrian-prantl
added a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Oct 16, 2024
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. (cherry picked from commit 089227f)
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
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.
JDevlieghere
added a commit
to JDevlieghere/llvm-project
that referenced
this pull request
Jan 30, 2025
Xcode uses a pseudoterminal for the debugger console. - The upside of this apporach is that it means that it can rely on LLDB's IOHandlers for multiline and script input. - The downside of this approach is that the command output is printed to the PTY and you don't get a SBCommandReturnObject. Adrian added support for inline diagnostics in llvm#110901 and we'd like to access those from the IDE. This patch adds support for registering a callback in the command interpreter that gives access to the (SB)CommandReturnObject right before it will be printed. The callback implementation can choose whether it likes to handle printing the result or defer to lldb. If the callback indicated it handled the result, the command interpreter will skip printing the result. We considered a few other alternatives to solve this problem: - The most obvious one is using `HandleCommand`, which returns a `SBCommandReturnObject`. The problem with this approach is the multiline input mentioned above. We would need a way to tell the IDE that it should expect multiline input, which isn't known until LLDB starts handling the command. - To address the multiline issue,we considered exposing (some of the) IOHandler machinery through the SB API. To solve this particular issue, that would require reimplementing a ton of logic that already exists today in the CommandInterpeter. Furthermore that seems like overkill compared to the proposed solution. rdar://141254310
JDevlieghere
added a commit
to JDevlieghere/llvm-project
that referenced
this pull request
Jan 30, 2025
Xcode uses a pseudoterminal for the debugger console. - The upside of this apporach is that it means that it can rely on LLDB's IOHandlers for multiline and script input. - The downside of this approach is that the command output is printed to the PTY and you don't get a SBCommandReturnObject. Adrian added support for inline diagnostics in llvm#110901 and we'd like to access those from the IDE. This patch adds support for registering a callback in the command interpreter that gives access to the (SB)CommandReturnObject right before it will be printed. The callback implementation can choose whether it likes to handle printing the result or defer to lldb. If the callback indicated it handled the result, the command interpreter will skip printing the result. We considered a few other alternatives to solve this problem: - The most obvious one is using `HandleCommand`, which returns a `SBCommandReturnObject`. The problem with this approach is the multiline input mentioned above. We would need a way to tell the IDE that it should expect multiline input, which isn't known until LLDB starts handling the command. - To address the multiline issue,we considered exposing (some of the) IOHandler machinery through the SB API. To solve this particular issue, that would require reimplementing a ton of logic that already exists today in the CommandInterpeter. Furthermore that seems like overkill compared to the proposed solution. rdar://141254310
JDevlieghere
added a commit
that referenced
this pull request
Feb 4, 2025
Xcode uses a pseudoterminal for the debugger console. - The upside of this apporach is that it means that it can rely on LLDB's IOHandlers for multiline and script input. - The downside of this approach is that the command output is printed to the PTY and you don't get a SBCommandReturnObject. Adrian added support for inline diagnostics (#110901) and we'd like to access those from the IDE. This patch adds support for registering a callback in the command interpreter that gives access to the `(SB)CommandReturnObject` right before it will be printed. The callback implementation can choose whether it likes to handle printing the result or defer to lldb. If the callback indicated it handled the result, the command interpreter will skip printing the result. We considered a few other alternatives to solve this problem: - The most obvious one is using `HandleCommand`, which returns a `SBCommandReturnObject`. The problem with this approach is the multiline input mentioned above. We would need a way to tell the IDE that it should expect multiline input, which isn't known until LLDB starts handling the command. - To address the multiline issue,we considered exposing (some of the) IOHandler machinery through the SB API. To solve this particular issue, that would require reimplementing a ton of logic that already exists today in the CommandInterpeter. Furthermore that seems like overkill compared to the proposed solution. rdar://141254310
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 5, 2025
Xcode uses a pseudoterminal for the debugger console. - The upside of this apporach is that it means that it can rely on LLDB's IOHandlers for multiline and script input. - The downside of this approach is that the command output is printed to the PTY and you don't get a SBCommandReturnObject. Adrian added support for inline diagnostics (llvm#110901) and we'd like to access those from the IDE. This patch adds support for registering a callback in the command interpreter that gives access to the `(SB)CommandReturnObject` right before it will be printed. The callback implementation can choose whether it likes to handle printing the result or defer to lldb. If the callback indicated it handled the result, the command interpreter will skip printing the result. We considered a few other alternatives to solve this problem: - The most obvious one is using `HandleCommand`, which returns a `SBCommandReturnObject`. The problem with this approach is the multiline input mentioned above. We would need a way to tell the IDE that it should expect multiline input, which isn't known until LLDB starts handling the command. - To address the multiline issue,we considered exposing (some of the) IOHandler machinery through the SB API. To solve this particular issue, that would require reimplementing a ton of logic that already exists today in the CommandInterpeter. Furthermore that seems like overkill compared to the proposed solution. rdar://141254310 (cherry picked from commit 97f6e53)
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
Xcode uses a pseudoterminal for the debugger console. - The upside of this apporach is that it means that it can rely on LLDB's IOHandlers for multiline and script input. - The downside of this approach is that the command output is printed to the PTY and you don't get a SBCommandReturnObject. Adrian added support for inline diagnostics (llvm#110901) and we'd like to access those from the IDE. This patch adds support for registering a callback in the command interpreter that gives access to the `(SB)CommandReturnObject` right before it will be printed. The callback implementation can choose whether it likes to handle printing the result or defer to lldb. If the callback indicated it handled the result, the command interpreter will skip printing the result. We considered a few other alternatives to solve this problem: - The most obvious one is using `HandleCommand`, which returns a `SBCommandReturnObject`. The problem with this approach is the multiline input mentioned above. We would need a way to tell the IDE that it should expect multiline input, which isn't known until LLDB starts handling the command. - To address the multiline issue,we considered exposing (some of the) IOHandler machinery through the SB API. To solve this particular issue, that would require reimplementing a ton of logic that already exists today in the CommandInterpeter. Furthermore that seems like overkill compared to the proposed solution. rdar://141254310
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.