[Lldb-commits] [lldb] Support inline diagnostics in CommandReturnObject (PR #110901)
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 10 18:16:00 PDT 2024
https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/110901
>From e13c841a6452ae5b0cfae129764dc95d6ac29e09 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 9 Oct 2024 09:45:53 -0700
Subject: [PATCH] Support inline diagnostics in CommandReturnObject
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.
---
.../lldb/Expression/DiagnosticManager.h | 38 ++--------
.../lldb/Interpreter/CommandReturnObject.h | 21 ++++--
lldb/include/lldb/Utility/Args.h | 6 +-
.../lldb/Utility/DiagnosticsRendering.h | 75 +++++++++++++++++++
lldb/include/lldb/Utility/Status.h | 12 +--
.../Commands/CommandObjectDWIMPrint.cpp | 9 +++
.../Commands/CommandObjectExpression.cpp | 5 +-
lldb/source/Expression/DiagnosticManager.cpp | 4 +-
lldb/source/Expression/FunctionCaller.cpp | 4 +-
lldb/source/Expression/LLVMUserExpression.cpp | 4 +-
.../source/Interpreter/CommandInterpreter.cpp | 22 +++++-
lldb/source/Interpreter/CommandObject.cpp | 11 +--
.../Interpreter/CommandReturnObject.cpp | 36 ++++++++-
lldb/source/Interpreter/Options.cpp | 49 +++++++++++-
lldb/source/Utility/Args.cpp | 41 ++++++----
lldb/source/Utility/CMakeLists.txt | 1 +
.../DiagnosticsRendering.cpp} | 59 ++++++++++-----
lldb/source/Utility/Status.cpp | 31 --------
.../Shell/Commands/command-dwim-print.test | 16 ++++
lldb/test/Shell/Commands/command-options.test | 16 ++++
lldb/unittests/Interpreter/CMakeLists.txt | 1 -
lldb/unittests/Utility/CMakeLists.txt | 1 +
.../DiagnosticsRenderingTest.cpp} | 6 +-
23 files changed, 322 insertions(+), 146 deletions(-)
create mode 100644 lldb/include/lldb/Utility/DiagnosticsRendering.h
rename lldb/source/{Commands/DiagnosticRendering.h => Utility/DiagnosticsRendering.cpp} (70%)
create mode 100644 lldb/test/Shell/Commands/command-dwim-print.test
create mode 100644 lldb/test/Shell/Commands/command-options.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..cc802b6f48334a 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,7 @@
#include "lldb/lldb-defines.h"
#include "lldb/lldb-types.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
@@ -23,49 +24,22 @@
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
- : public llvm::ErrorInfo<ExpressionError, ExpressionErrorBase> {
+ : public llvm::ErrorInfo<ExpressionError, DiagnosticError> {
std::string m_message;
std::vector<DiagnosticDetail> m_details;
public:
static char ID;
- using llvm::ErrorInfo<ExpressionError, ExpressionErrorBase>::ErrorInfo;
+ using llvm::ErrorInfo<ExpressionError, DiagnosticError>::ErrorInfo;
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..e13c3b7b8e0437 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -10,6 +10,7 @@
#define LLDB_INTERPRETER_COMMANDRETURNOBJECT_H
#include "lldb/Host/StreamFile.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StreamTee.h"
#include "lldb/lldb-private.h"
@@ -29,6 +30,8 @@ class CommandReturnObject {
~CommandReturnObject() = default;
+ llvm::StringRef GetInlineDiagnosticsData(unsigned indent);
+
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,14 @@ class CommandReturnObject {
void SetError(llvm::Error error);
+ void SetDiagnosticIndent(std::optional<uint16_t> indent) {
+ m_diagnostic_indent = indent;
+ }
+
+ std::optional<uint16_t> GetDiagnosticIndent() const {
+ return m_diagnostic_indent;
+ }
+
lldb::ReturnStatus GetStatus() const;
void SetStatus(lldb::ReturnStatus status);
@@ -160,6 +166,9 @@ class CommandReturnObject {
StreamTee m_out_stream;
StreamTee m_err_stream;
+ std::vector<DiagnosticDetail> m_diagnostics;
+ StreamString m_diag_stream;
+ std::optional<uint16_t> m_diagnostic_indent;
lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
diff --git a/lldb/include/lldb/Utility/Args.h b/lldb/include/lldb/Utility/Args.h
index 757f7e2ba5ec67..094bb290691198 100644
--- a/lldb/include/lldb/Utility/Args.h
+++ b/lldb/include/lldb/Utility/Args.h
@@ -38,12 +38,14 @@ class Args {
std::unique_ptr<char[]> ptr;
char quote = '\0';
+ /// The position of the argument in the original argument string.
+ std::optional<uint16_t> column;
char *data() { return ptr.get(); }
public:
ArgEntry() = default;
- ArgEntry(llvm::StringRef str, char quote);
+ ArgEntry(llvm::StringRef str, char quote, std::optional<uint16_t> column);
llvm::StringRef ref() const { return c_str(); }
const char *c_str() const { return ptr.get(); }
@@ -51,6 +53,8 @@ class Args {
/// Returns true if this argument was quoted in any way.
bool IsQuoted() const { return quote != '\0'; }
char GetQuoteChar() const { return quote; }
+ std::optional<uint16_t> GetPos() const { return column; }
+ size_t GetLength() const { return ref().size(); }
};
/// Construct with an option command string.
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h b/lldb/include/lldb/Utility/DiagnosticsRendering.h
new file mode 100644
index 00000000000000..33aded602e3a77
--- /dev/null
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -0,0 +1,75 @@
+//===-- 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 {
+
+/// A compiler-independent representation of an \c
+/// lldb_private::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 {
+ /// A source location consisting of a file name and position.
+ struct SourceLocation {
+ /// \c "<user expression 0>" in the example above.
+ FileSpec file;
+ /// \c 1 in the example above.
+ unsigned line = 0;
+ /// \c 5 in the example above.
+ uint16_t column = 0;
+ /// \c 3 in the example above.
+ uint16_t length = 0;
+ /// Whether this source location should be surfaced to the
+ /// user. For example, syntax errors diagnosed in LLDB's
+ /// expression wrapper code have this set to true.
+ bool hidden = false;
+ /// Whether this source location refers to something the user
+ /// typed as part of the command, i.e., if this qualifies for
+ /// inline display, or if the source line would need to be echoed
+ /// again for the message to make sense.
+ bool in_user_input = false;
+ };
+ /// Contains this diagnostic's source location, if applicable.
+ std::optional<SourceLocation> source_location;
+ /// Contains \c 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, without "error: ",
+ /// but including the source context.
+ std::string rendered;
+};
+
+class DiagnosticError
+ : public llvm::ErrorInfo<DiagnosticError, CloneableECError> {
+public:
+ using llvm::ErrorInfo<DiagnosticError, CloneableECError>::ErrorInfo;
+ DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
+ lldb::ErrorType GetErrorType() const override;
+ virtual llvm::ArrayRef<DiagnosticDetail> GetDetails() const = 0;
+ static char ID;
+};
+
+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 2911ffb804c6fb..b8993dff3bb18a 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,8 +27,6 @@ class raw_ostream;
namespace lldb_private {
-const char *ExpressionResultAsCString(lldb::ExpressionResults result);
-
/// Going a bit against the spirit of llvm::Error,
/// lldb_private::Status need to store errors long-term and sometimes
/// copy them. This base class defines an interface for this
@@ -79,15 +78,6 @@ class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
static char ID;
};
-class ExpressionErrorBase
- : public llvm::ErrorInfo<ExpressionErrorBase, CloneableECError> {
-public:
- using llvm::ErrorInfo<ExpressionErrorBase, CloneableECError>::ErrorInfo;
- ExpressionErrorBase(std::error_code ec) : ErrorInfo(ec) {}
- lldb::ErrorType GetErrorType() const override;
- static char ID;
-};
-
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 2e1d6e6e5af996..ae620e8ff3d8b9 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -191,6 +191,15 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
ExpressionResults expr_result = target.EvaluateExpression(
expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
+ // Record the position of the expression in the command.
+ std::optional<uint16_t> indent;
+ if (fixed_expression.empty()) {
+ size_t pos = m_original_command.find(expr);
+ if (pos != llvm::StringRef::npos)
+ indent = pos;
+ }
+ result.SetDiagnosticIndent(indent);
+
// 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()) {
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index a72b409d21ed83..8effb1a5988370 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(); }));
+ [&](DiagnosticError &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/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index 7c67a0ce4aa026..b1fa14bccc17bd 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -10,6 +10,7 @@
#include "llvm/Support/ErrorHandling.h"
+#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/StreamString.h"
@@ -22,8 +23,7 @@ class ExpressionCategory : public std::error_category {
return "LLDBExpressionCategory";
}
std::string message(int __ev) const override {
- return ExpressionResultAsCString(
- static_cast<lldb::ExpressionResults>(__ev));
+ return toString(static_cast<lldb::ExpressionResults>(__ev));
};
};
ExpressionCategory &expression_category() {
diff --git a/lldb/source/Expression/FunctionCaller.cpp b/lldb/source/Expression/FunctionCaller.cpp
index 5ce0175fedf453..d1dd350b09f25e 100644
--- a/lldb/source/Expression/FunctionCaller.cpp
+++ b/lldb/source/Expression/FunctionCaller.cpp
@@ -6,7 +6,6 @@
//
//===----------------------------------------------------------------------===//
-
#include "lldb/Expression/FunctionCaller.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ValueObject.h"
@@ -24,6 +23,7 @@
#include "lldb/Target/ThreadPlan.h"
#include "lldb/Target/ThreadPlanCallFunction.h"
#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/State.h"
@@ -390,7 +390,7 @@ lldb::ExpressionResults FunctionCaller::ExecuteFunction(
LLDB_LOGF(log,
"== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
"completed abnormally: %s ==",
- m_name.c_str(), ExpressionResultAsCString(return_value));
+ m_name.c_str(), toString(return_value).c_str());
} else {
LLDB_LOGF(log,
"== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index 9e07d4a7374a78..d76f04d9b86e2e 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -6,7 +6,6 @@
//
//===----------------------------------------------------------------------===//
-
#include "lldb/Expression/LLVMUserExpression.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ValueObjectConstResult.h"
@@ -30,6 +29,7 @@
#include "lldb/Target/ThreadPlan.h"
#include "lldb/Target/ThreadPlanCallUserExpression.h"
#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/StreamString.h"
@@ -237,7 +237,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
} else if (execution_result != lldb::eExpressionCompleted) {
diagnostic_manager.Printf(lldb::eSeverityError,
"Couldn't execute function; result was %s",
- ExpressionResultAsCString(execution_result));
+ toString(execution_result).c_str());
return execution_result;
}
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 8d3a82ef6c990a..313ab8123ee590 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());
@@ -2078,6 +2078,11 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
ElapsedTime elapsed(execute_time);
cmd_obj->SetOriginalCommandString(real_original_command_string);
+ pos = real_original_command_string.rfind(remainder);
+ std::optional<uint16_t> indent;
+ if (pos != std::string::npos)
+ indent = pos;
+ result.SetDiagnosticIndent(indent);
cmd_obj->Execute(remainder.c_str(), result);
}
@@ -3180,7 +3185,18 @@ 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
+ // Display any inline diagnostics first.
+ if (!result.GetImmediateErrorStream() &&
+ GetDebugger().GetShowInlineDiagnostics()) {
+ unsigned prompt_len = m_debugger.GetPrompt().size();
+ if (auto indent = result.GetDiagnosticIndent()) {
+ llvm::StringRef diags =
+ result.GetInlineDiagnosticsData(prompt_len + *indent);
+ PrintCommandOutput(io_handler, diags, true);
+ }
+ }
+
+ // Display any STDOUT/STDERR _prior_ to emitting the command result text.
GetProcessOutput();
if (!result.GetImmediateOutputStream()) {
@@ -3188,7 +3204,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
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/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp
index cf2682cd26faa0..6b044a28eb37ee 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -127,16 +127,7 @@ bool CommandObject::ParseOptions(Args &args, CommandReturnObject &result) {
if (options->VerifyOptions(result))
return true;
} else {
- const char *error_cstr = error.AsCString();
- if (error_cstr) {
- // We got an error string, lets use that
- result.AppendError(error_cstr);
- } else {
- // No error string, output the usage information into result
- options->GenerateOptionUsage(
- result.GetErrorStream(), *this,
- GetCommandInterpreter().GetDebugger().GetTerminalWidth());
- }
+ result.SetError(error.takeError());
}
result.SetStatus(eReturnStatusFailed);
return false;
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index d5da73c00a5209..7c9905bc57d992 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"
@@ -41,7 +42,7 @@ static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
}
CommandReturnObject::CommandReturnObject(bool colors)
- : m_out_stream(colors), m_err_stream(colors) {}
+ : m_out_stream(colors), m_err_stream(colors), m_diag_stream(colors) {}
void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) {
SetStatus(eReturnStatusFailed);
@@ -112,8 +113,39 @@ void CommandReturnObject::SetError(Status error) {
}
void CommandReturnObject::SetError(llvm::Error error) {
- if (error)
+ // Retrieve any diagnostics.
+ error = llvm::handleErrors(std::move(error), [&](DiagnosticError &error) {
+ SetStatus(eReturnStatusFailed);
+ m_diagnostics = error.GetDetails();
+ });
+ if (error) {
AppendError(llvm::toString(std::move(error)));
+ }
+}
+
+llvm::StringRef CommandReturnObject::GetInlineDiagnosticsData(unsigned indent) {
+ 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/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp
index 3888a5812628cd..b9a911c2007152 100644
--- a/lldb/source/Interpreter/Options.cpp
+++ b/lldb/source/Interpreter/Options.cpp
@@ -19,12 +19,47 @@
#include "lldb/Interpreter/CommandObject.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Target/Target.h"
+#include "lldb/Utility/DiagnosticsRendering.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/STLExtras.h"
using namespace lldb;
using namespace lldb_private;
+namespace lldb_private {
+
+/// An llvm::Error that represents an option parsing diagnostic.
+class OptionParseError
+ : public llvm::ErrorInfo<OptionParseError, DiagnosticError> {
+ std::vector<DiagnosticDetail> m_details;
+
+public:
+ using llvm::ErrorInfo<OptionParseError, DiagnosticError>::ErrorInfo;
+ OptionParseError(DiagnosticDetail detail)
+ : ErrorInfo(std::error_code(EINVAL, std::generic_category())),
+ m_details({detail}) {}
+ OptionParseError(const Args::ArgEntry &arg, std::string msg)
+ : ErrorInfo(std::error_code(EINVAL, std::generic_category())) {
+ DiagnosticDetail::SourceLocation sloc;
+ if (auto pos = arg.GetPos()) {
+ uint16_t len = arg.GetLength();
+ sloc = {FileSpec{}, 1, *pos, len, false, true};
+ }
+ m_details.push_back(DiagnosticDetail{sloc, lldb::eSeverityError, msg, msg});
+ }
+ std::unique_ptr<CloneableError> Clone() const override {
+ return std::make_unique<OptionParseError>(m_details[0]);
+ }
+ llvm::ArrayRef<DiagnosticDetail> GetDetails() const override {
+ return m_details;
+ }
+ static char ID;
+};
+
+char OptionParseError::ID;
+
+} // namespace lldb_private
+
// Options
Options::Options() { BuildValidOptionSets(); }
@@ -1254,13 +1289,13 @@ llvm::Expected<Args> Options::Parse(const Args &args,
std::string short_options = BuildShortOptions(long_options);
std::vector<char *> argv = GetArgvForParsing(args);
+
std::unique_lock<std::mutex> lock;
OptionParser::Prepare(lock);
- int val;
while (true) {
int long_options_index = -1;
- val = OptionParser::Parse(argv, short_options, long_options,
- &long_options_index);
+ int val = OptionParser::Parse(argv, short_options, long_options,
+ &long_options_index);
if (val == ':') {
error = Status::FromErrorString("last option requires an argument");
@@ -1272,7 +1307,13 @@ llvm::Expected<Args> Options::Parse(const Args &args,
// Did we get an error?
if (val == '?') {
- error = Status::FromErrorString("unknown or ambiguous option");
+ int idx = OptionParser::GetOptionIndex() - 2;
+ if (idx >= 0 && (size_t)idx < args.GetArgumentCount())
+ error = Status::FromError(llvm::make_error<OptionParseError>(
+ args[idx], "unknown or ambiguous option"));
+ else
+ error = Status("unknown or ambiguous option");
+
break;
}
// The option auto-set itself
diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 13b993bc74c9f4..8ba40bae4d67e5 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -66,9 +66,11 @@ static size_t ArgvToArgc(const char **argv) {
// Trims all whitespace that can separate command line arguments from the left
// side of the string.
-static llvm::StringRef ltrimForArgs(llvm::StringRef str) {
+static llvm::StringRef ltrimForArgs(llvm::StringRef str, size_t &shift) {
static const char *k_space_separators = " \t";
- return str.ltrim(k_space_separators);
+ llvm::StringRef result = str.ltrim(k_space_separators);
+ shift = result.data() - str.data();
+ return result;
}
// A helper function for SetCommandString. Parses a single argument from the
@@ -156,7 +158,9 @@ ParseSingleArgument(llvm::StringRef command) {
return std::make_tuple(arg, first_quote_char, command);
}
-Args::ArgEntry::ArgEntry(llvm::StringRef str, char quote) : quote(quote) {
+Args::ArgEntry::ArgEntry(llvm::StringRef str, char quote,
+ std::optional<uint16_t> column)
+ : quote(quote), column(column) {
size_t size = str.size();
ptr.reset(new char[size + 1]);
@@ -185,7 +189,7 @@ Args &Args::operator=(const Args &rhs) {
m_argv.clear();
m_entries.clear();
for (auto &entry : rhs.m_entries) {
- m_entries.emplace_back(entry.ref(), entry.quote);
+ m_entries.emplace_back(entry.ref(), entry.quote, entry.column);
m_argv.push_back(m_entries.back().data());
}
m_argv.push_back(nullptr);
@@ -248,14 +252,20 @@ void Args::SetCommandString(llvm::StringRef command) {
Clear();
m_argv.clear();
- command = ltrimForArgs(command);
+ uint16_t column = 1;
+ size_t shift = 0;
+ command = ltrimForArgs(command, shift);
+ column += shift;
std::string arg;
char quote;
while (!command.empty()) {
+ const char *prev = command.data();
std::tie(arg, quote, command) = ParseSingleArgument(command);
- m_entries.emplace_back(arg, quote);
+ m_entries.emplace_back(arg, quote, column);
m_argv.push_back(m_entries.back().data());
- command = ltrimForArgs(command);
+ command = ltrimForArgs(command, shift);
+ column += shift;
+ column += command.data() - prev;
}
m_argv.push_back(nullptr);
}
@@ -299,7 +309,7 @@ void Args::AppendArguments(const Args &rhs) {
assert(m_argv.back() == nullptr);
m_argv.pop_back();
for (auto &entry : rhs.m_entries) {
- m_entries.emplace_back(entry.ref(), entry.quote);
+ m_entries.emplace_back(entry.ref(), entry.quote, entry.column);
m_argv.push_back(m_entries.back().data());
}
m_argv.push_back(nullptr);
@@ -312,7 +322,7 @@ void Args::AppendArguments(const char **argv) {
assert(m_argv.back() == nullptr);
m_argv.pop_back();
for (auto arg : llvm::ArrayRef(argv, argc)) {
- m_entries.emplace_back(arg, '\0');
+ m_entries.emplace_back(arg, '\0', std::nullopt);
m_argv.push_back(m_entries.back().data());
}
@@ -330,7 +340,7 @@ void Args::InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str,
if (idx > m_entries.size())
return;
- m_entries.emplace(m_entries.begin() + idx, arg_str, quote_char);
+ m_entries.emplace(m_entries.begin() + idx, arg_str, quote_char, std::nullopt);
m_argv.insert(m_argv.begin() + idx, m_entries[idx].data());
}
@@ -342,7 +352,7 @@ void Args::ReplaceArgumentAtIndex(size_t idx, llvm::StringRef arg_str,
if (idx >= m_entries.size())
return;
- m_entries[idx] = ArgEntry(arg_str, quote_char);
+ m_entries[idx] = ArgEntry(arg_str, quote_char, std::nullopt);
m_argv[idx] = m_entries[idx].data();
}
@@ -366,7 +376,7 @@ void Args::SetArguments(size_t argc, const char **argv) {
? args[i][0]
: '\0';
- m_entries[i] = ArgEntry(args[i], quote);
+ m_entries[i] = ArgEntry(args[i], quote, std::nullopt);
m_argv[i] = m_entries[i].data();
}
}
@@ -635,7 +645,8 @@ OptionsWithRaw::OptionsWithRaw(llvm::StringRef arg_string) {
void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) {
const llvm::StringRef original_args = arg_string;
- arg_string = ltrimForArgs(arg_string);
+ size_t shift;
+ arg_string = ltrimForArgs(arg_string, shift);
std::string arg;
char quote;
@@ -656,7 +667,7 @@ void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) {
// If we get an unquoted '--' argument, then we reached the suffix part
// of the command.
- Args::ArgEntry entry(arg, quote);
+ Args::ArgEntry entry(arg, quote, std::nullopt);
if (!entry.IsQuoted() && arg == "--") {
// The remaining line is the raw suffix, and the line we parsed so far
// needs to be interpreted as arguments.
@@ -681,7 +692,7 @@ void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) {
break;
}
- arg_string = ltrimForArgs(arg_string);
+ arg_string = ltrimForArgs(arg_string, shift);
}
// If we didn't find a suffix delimiter, the whole string is the raw suffix.
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 70%
rename from lldb/source/Commands/DiagnosticRendering.h
rename to lldb/source/Utility/DiagnosticsRendering.cpp
index 5fdd090253a827..96caf934cc2315 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,15 +6,19 @@
//
//===----------------------------------------------------------------------===//
-#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 {
+char DiagnosticError::ID;
+
+lldb::ErrorType DiagnosticError::GetErrorType() const {
+ return lldb::eErrorTypeExpression;
+}
+
static llvm::raw_ostream &PrintSeverity(Stream &stream,
lldb::Severity severity) {
llvm::HighlightColor color;
@@ -36,12 +40,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;
@@ -53,6 +56,27 @@ static void RenderDiagnosticDetails(Stream &stream,
return;
}
+ // Since there is no other way to find this out, use the color
+ // attribute as a proxy for whether the terminal supports Unicode
+ // characters. In the future it might make sense to move this into
+ // Host so it can be customized for a specific platform.
+ llvm::StringRef cursor, underline, vbar, joint, hbar, spacer;
+ if (stream.AsRawOstream().colors_enabled()) {
+ cursor = "˄";
+ underline = "˜";
+ vbar = "│";
+ joint = "╰";
+ hbar = "─";
+ spacer = " ";
+ } else {
+ cursor = "^";
+ underline = "~";
+ vbar = "|";
+ joint = "";
+ hbar = "";
+ spacer = "";
+ }
+
// Print a line with caret indicator(s) below the lldb prompt + command.
const size_t padding = *offset_in_command;
stream << std::string(padding, ' ');
@@ -78,9 +102,9 @@ static void RenderDiagnosticDetails(Stream &stream,
remaining_details.push_back(detail);
if (offset > loc.column)
continue;
- stream << std::string(loc.column - offset, ' ') << '^';
- if (loc.length > 1)
- stream << std::string(loc.length - 1, '~');
+ stream << std::string(loc.column - offset, ' ') << cursor;
+ for (unsigned i = 0; i + 1 < loc.length; ++i)
+ stream << underline;
offset = loc.column + 1;
}
stream << '\n';
@@ -97,14 +121,14 @@ static void RenderDiagnosticDetails(Stream &stream,
for (auto &remaining_detail :
llvm::ArrayRef(remaining_details).drop_back(1)) {
uint16_t column = remaining_detail.source_location->column;
- stream << std::string(column - offset, ' ') << "│";
+ stream << std::string(column - offset, ' ') << vbar;
offset = column + 1;
}
// Print the line connecting the ^ with the error message.
uint16_t column = detail->source_location->column;
if (offset <= column)
- stream << std::string(column - offset, ' ') << "╰─ ";
+ stream << std::string(column - offset, ' ') << joint << hbar << spacer;
// Print a colorized string based on the message's severity type.
PrintSeverity(stream, detail->severity);
@@ -130,4 +154,3 @@ static void RenderDiagnosticDetails(Stream &stream,
}
} // namespace lldb_private
-#endif
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index cf3772bc480ba3..1d171c6b6c3746 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -43,7 +43,6 @@ char CloneableError::ID;
char CloneableECError::ID;
char MachKernelError::ID;
char Win32Error::ID;
-char ExpressionErrorBase::ID;
namespace {
/// A std::error_code category for eErrorTypeGeneric.
@@ -253,10 +252,6 @@ lldb::ErrorType Win32Error::GetErrorType() const {
return lldb::eErrorTypeWin32;
}
-lldb::ErrorType ExpressionErrorBase::GetErrorType() const {
- return lldb::eErrorTypeExpression;
-}
-
ErrorType Status::GetType() const {
ErrorType result = eErrorTypeInvalid;
llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) {
@@ -286,29 +281,3 @@ void llvm::format_provider<lldb_private::Status>::format(
llvm::format_provider<llvm::StringRef>::format(error.AsCString(), OS,
Options);
}
-
-const char *lldb_private::ExpressionResultAsCString(ExpressionResults result) {
- switch (result) {
- case eExpressionCompleted:
- return "eExpressionCompleted";
- case eExpressionDiscarded:
- return "eExpressionDiscarded";
- case eExpressionInterrupted:
- return "eExpressionInterrupted";
- case eExpressionHitBreakpoint:
- return "eExpressionHitBreakpoint";
- case eExpressionSetupError:
- return "eExpressionSetupError";
- case eExpressionParseError:
- return "eExpressionParseError";
- case eExpressionResultUnavailable:
- return "eExpressionResultUnavailable";
- case eExpressionTimedOut:
- return "eExpressionTimedOut";
- case eExpressionStoppedForDebug:
- return "eExpressionStoppedForDebug";
- case eExpressionThreadVanished:
- return "eExpressionThreadVanished";
- }
- return "<unknown>";
-}
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..9153edbd217913
--- /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/test/Shell/Commands/command-options.test b/lldb/test/Shell/Commands/command-options.test
new file mode 100644
index 00000000000000..73aa374bde297d
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-options.test
@@ -0,0 +1,16 @@
+# RUN: echo quit | %lldb -O "log enable -x" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK1
+# (lldb) log enable -x
+# CHECK1:{{^ \^~}}
+# CHECK1: {{^ error: unknown or ambiguous option}}
+
+# RUN: echo quit | %lldb -O " log enable -xxxxxxx" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK2
+# (lldb) log enable -xxxxxxx
+# CHECK2:{{^ \^~~~~~~~}}
+# CHECK2: {{^ error: unknown or ambiguous option}}
+# RUN: echo quit | %lldb -O "log enable dwarf all -f dwarf.log -x" \
+# RUN: | FileCheck %s --strict-whitespace --check-prefix=CHECK3
+# (lldb) log enable dwarf all -f dwarf.log -x
+# CHECK3:{{^ \^~}}
+# CHECK3: {{^ error: unknown or ambiguous option}}
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;
More information about the lldb-commits
mailing list