[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 26 12:53:00 PDT 2024
https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/106470
>From 9f4ba3fdb8b144736e51134ced3019a2c57ca861 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 28 Aug 2024 10:04:33 -0700
Subject: [PATCH 1/2] [lldb] Store expression evaluator diagnostics in an
llvm::Error (NFC)
This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938
This patch is preparatory patch for improving the rendering of
expression evaluator diagnostics. Currently diagnostics are rendered
into a string and the command interpreter layer then textually parses
words like "error:" to (sometimes) color the output accordingly. In
order to enable user interfaces to do better with diagnostics, we need
to store them in a machine-readable fromat. This patch does this by
adding a new llvm::Error kind wrapping a DiagnosticDetail struct that
is used when the error type is eErrorTypeExpression. Multiple
diagnostics are modeled using llvm::ErrorList.
Right now the extra information is not used by the CommandInterpreter,
this will be added in a follow-up patch!
---
.../lldb/Expression/DiagnosticManager.h | 89 +++++++++++----
lldb/include/lldb/Utility/Status.h | 23 ++--
lldb/source/Breakpoint/BreakpointLocation.cpp | 11 +-
lldb/source/Expression/DiagnosticManager.cpp | 103 +++++++++++++++---
lldb/source/Expression/ExpressionParser.cpp | 5 +-
lldb/source/Expression/UserExpression.cpp | 49 +++++----
lldb/source/Expression/UtilityFunction.cpp | 18 +--
.../ExpressionParser/Clang/ClangDiagnostic.h | 5 +-
.../Clang/ClangExpressionParser.cpp | 69 ++++++++----
.../Clang/ClangUserExpression.h | 2 +
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 12 +-
.../Platform/Windows/PlatformWindows.cpp | 12 +-
lldb/source/Target/Target.cpp | 3 +-
lldb/source/Utility/Status.cpp | 29 +----
.../TestModulesCompileError.py | 2 +-
.../Expression/DiagnosticManagerTest.cpp | 22 +++-
16 files changed, 292 insertions(+), 162 deletions(-)
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..3e363ee5c0139c 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,9 @@
#include "lldb/lldb-defines.h"
#include "lldb/lldb-types.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -20,6 +23,53 @@
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 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> {
+ std::string m_message;
+ std::vector<DiagnosticDetail> m_details;
+
+public:
+ static char ID;
+ using llvm::ErrorInfo<ExpressionError, ExpressionErrorBase>::ErrorInfo;
+ ExpressionError(lldb::ExpressionResults result, std::string msg,
+ std::vector<DiagnosticDetail> details = {});
+ std::string message() const override;
+ llvm::ArrayRef<DiagnosticDetail> GetDetail() const { return m_details; }
+ std::error_code convertToErrorCode() const override;
+ void log(llvm::raw_ostream &OS) const override;
+ std::unique_ptr<CloneableError> Clone() const override;
+};
+
enum DiagnosticOrigin {
eDiagnosticOriginUnknown = 0,
eDiagnosticOriginLLDB,
@@ -49,37 +99,28 @@ class Diagnostic {
}
}
- Diagnostic(llvm::StringRef message, lldb::Severity severity,
- DiagnosticOrigin origin, uint32_t compiler_id)
- : m_message(message), m_severity(severity), m_origin(origin),
- m_compiler_id(compiler_id) {}
-
- Diagnostic(const Diagnostic &rhs)
- : m_message(rhs.m_message), m_severity(rhs.m_severity),
- m_origin(rhs.m_origin), m_compiler_id(rhs.m_compiler_id) {}
+ Diagnostic(DiagnosticOrigin origin, uint32_t compiler_id,
+ DiagnosticDetail detail)
+ : m_origin(origin), m_compiler_id(compiler_id), m_detail(detail) {}
virtual ~Diagnostic() = default;
virtual bool HasFixIts() const { return false; }
- lldb::Severity GetSeverity() const { return m_severity; }
+ lldb::Severity GetSeverity() const { return m_detail.severity; }
uint32_t GetCompilerID() const { return m_compiler_id; }
- llvm::StringRef GetMessage() const { return m_message; }
+ llvm::StringRef GetMessage() const { return m_detail.message; }
+ const DiagnosticDetail &GetDetail() const { return m_detail; }
- void AppendMessage(llvm::StringRef message,
- bool precede_with_newline = true) {
- if (precede_with_newline)
- m_message.push_back('\n');
- m_message += message;
- }
+ void AppendMessage(llvm::StringRef message, bool precede_with_newline = true);
protected:
- std::string m_message;
- lldb::Severity m_severity;
DiagnosticOrigin m_origin;
- uint32_t m_compiler_id; // Compiler-specific diagnostic ID
+ /// Compiler-specific diagnostic ID.
+ uint32_t m_compiler_id;
+ DiagnosticDetail m_detail;
};
typedef std::vector<std::unique_ptr<Diagnostic>> DiagnosticList;
@@ -102,10 +143,7 @@ class DiagnosticManager {
void AddDiagnostic(llvm::StringRef message, lldb::Severity severity,
DiagnosticOrigin origin,
- uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
- m_diagnostics.emplace_back(
- std::make_unique<Diagnostic>(message, severity, origin, compiler_id));
- }
+ uint32_t compiler_id = LLDB_INVALID_COMPILER_ID);
void AddDiagnostic(std::unique_ptr<Diagnostic> diagnostic) {
if (diagnostic)
@@ -130,6 +168,11 @@ class DiagnosticManager {
m_diagnostics.back()->AppendMessage(str);
}
+ /// Copies the diagnostics into an llvm::Error{List}.
+ /// The first diagnostic wraps \c result.
+ llvm::Error GetAsError(lldb::ExpressionResults result,
+ llvm::Twine message = {}) const;
+
// Returns a string containing errors in this format:
//
// "error: error text\n
diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h
index 084ce4afb8cefd..64eedc868d10f0 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -75,18 +75,13 @@ class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
static char ID;
};
-class ExpressionError
- : public llvm::ErrorInfo<ExpressionError, CloneableECError> {
+class ExpressionErrorBase
+ : public llvm::ErrorInfo<ExpressionErrorBase, CloneableECError> {
public:
- using llvm::ErrorInfo<ExpressionError, CloneableECError>::ErrorInfo;
- ExpressionError(std::error_code ec, std::string msg = {})
- : ErrorInfo(ec), m_string(msg) {}
- std::unique_ptr<CloneableError> Clone() const override;
- std::string message() const override { return m_string; }
+ using llvm::ErrorInfo<ExpressionErrorBase, CloneableECError>::ErrorInfo;
+ ExpressionErrorBase(std::error_code ec, std::string msg = {})
+ : ErrorInfo(ec) {}
static char ID;
-
-protected:
- std::string m_string;
};
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
@@ -160,9 +155,6 @@ class Status {
return Status(llvm::formatv(format, std::forward<Args>(args)...));
}
- static Status FromExpressionError(lldb::ExpressionResults result,
- std::string msg);
-
/// Set the current error to errno.
///
/// Update the error value to be \c errno and update the type to be \c
@@ -175,8 +167,11 @@ class Status {
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
static Status FromError(llvm::Error error);
- /// FIXME: Replace this with a takeError() method.
+ /// FIXME: Replace all uses with takeError() instead.
llvm::Error ToError() const;
+
+ llvm::Error takeError() { return std::move(m_error); }
+
/// Don't call this function in new code. Instead, redesign the API
/// to use llvm::Expected instead of Status.
Status Clone() const { return Status(ToError()); }
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 8d7364052a006a..35058a713aef86 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -264,9 +264,10 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
if (!m_user_expression_sp->Parse(diagnostics, exe_ctx,
eExecutionPolicyOnlyWhenNeeded, true,
false)) {
- error = Status::FromErrorStringWithFormat(
- "Couldn't parse conditional expression:\n%s",
- diagnostics.GetString().c_str());
+ error = Status::FromError(
+ diagnostics.GetAsError(lldb::eExpressionParseError,
+ "Couldn't parse conditional expression:"));
+
m_user_expression_sp.reset();
return true;
}
@@ -324,8 +325,8 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
}
} else {
ret = false;
- error = Status::FromErrorStringWithFormat(
- "Couldn't execute expression:\n%s", diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ lldb::eExpressionParseError, "Couldn't execute expression:"));
}
return ret;
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index a8330138f3d53b..3731a200c5e2f7 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -14,23 +14,29 @@
#include "lldb/Utility/StreamString.h"
using namespace lldb_private;
+char ExpressionError::ID;
-void DiagnosticManager::Dump(Log *log) {
- if (!log)
- return;
-
- std::string str = GetString();
-
- // GetString() puts a separator after each diagnostic. We want to remove the
- // last '\n' because log->PutCString will add one for us.
-
- if (str.size() && str.back() == '\n') {
- str.pop_back();
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+ const char *name() const noexcept override {
+ return "LLDBExpressionCategory";
}
-
- log->PutCString(str.c_str());
+ std::string message(int __ev) const override {
+ return ExpressionResultAsCString(
+ static_cast<lldb::ExpressionResults>(__ev));
+ };
+};
+ExpressionCategory &expression_category() {
+ static ExpressionCategory g_expression_category;
+ return g_expression_category;
}
+ExpressionError::ExpressionError(lldb::ExpressionResults result,
+ std::string msg,
+ std::vector<DiagnosticDetail> details)
+ : ErrorInfo(std::error_code(result, expression_category())), m_message(msg),
+ m_details(details) {}
+
static const char *StringForSeverity(lldb::Severity severity) {
switch (severity) {
// this should be exhaustive
@@ -44,9 +50,33 @@ static const char *StringForSeverity(lldb::Severity severity) {
llvm_unreachable("switch needs another case for lldb::Severity enum");
}
+std::string ExpressionError::message() const {
+ std::string str;
+ {
+ llvm::raw_string_ostream os(str);
+ if (!m_message.empty())
+ os << m_message << '\n';
+ for (const auto &detail : m_details)
+ os << StringForSeverity(detail.severity) << detail.rendered << '\n';
+ }
+ return str;
+}
+
+std::error_code ExpressionError::convertToErrorCode() const {
+ return llvm::inconvertibleErrorCode();
+}
+
+void ExpressionError::log(llvm::raw_ostream &OS) const { OS << message(); }
+
+std::unique_ptr<CloneableError> ExpressionError::Clone() const {
+ return std::make_unique<ExpressionError>(
+ (lldb::ExpressionResults)convertToErrorCode().value(), m_message,
+ m_details);
+}
+
std::string DiagnosticManager::GetString(char separator) {
- std::string ret;
- llvm::raw_string_ostream stream(ret);
+ std::string str;
+ llvm::raw_string_ostream stream(str);
for (const auto &diagnostic : Diagnostics()) {
llvm::StringRef severity = StringForSeverity(diagnostic->GetSeverity());
@@ -61,8 +91,39 @@ std::string DiagnosticManager::GetString(char separator) {
stream << message.drop_front(severity_pos + severity.size());
stream << separator;
}
+ return str;
+}
+
+void DiagnosticManager::Dump(Log *log) {
+ if (!log)
+ return;
- return ret;
+ std::string str = GetString();
+
+ // We want to remove the last '\n' because log->PutCString will add
+ // one for us.
+
+ if (!str.empty() && str.back() == '\n')
+ str.pop_back();
+
+ log->PutString(str);
+}
+
+llvm::Error DiagnosticManager::GetAsError(lldb::ExpressionResults result,
+ llvm::Twine message) const {
+ std::vector<DiagnosticDetail> details;
+ for (const auto &diag : m_diagnostics)
+ details.push_back(diag->GetDetail());
+ return llvm::make_error<ExpressionError>(result, message.str(), details);
+}
+
+void DiagnosticManager::AddDiagnostic(llvm::StringRef message,
+ lldb::Severity severity,
+ DiagnosticOrigin origin,
+ uint32_t compiler_id) {
+ m_diagnostics.emplace_back(std::make_unique<Diagnostic>(
+ origin, compiler_id,
+ DiagnosticDetail{{}, severity, message.str(), message.str()}));
}
size_t DiagnosticManager::Printf(lldb::Severity severity, const char *format,
@@ -85,3 +146,13 @@ void DiagnosticManager::PutString(lldb::Severity severity,
return;
AddDiagnostic(str, severity, eDiagnosticOriginLLDB);
}
+
+void Diagnostic::AppendMessage(llvm::StringRef message,
+ bool precede_with_newline) {
+ if (precede_with_newline) {
+ m_detail.message.push_back('\n');
+ m_detail.rendered.push_back('\n');
+ }
+ m_detail.message += message;
+ m_detail.rendered += message;
+}
diff --git a/lldb/source/Expression/ExpressionParser.cpp b/lldb/source/Expression/ExpressionParser.cpp
index 868556c1c58a57..1ba5e10d65d058 100644
--- a/lldb/source/Expression/ExpressionParser.cpp
+++ b/lldb/source/Expression/ExpressionParser.cpp
@@ -63,9 +63,8 @@ ExpressionParser::RunStaticInitializers(IRExecutionUnitSP &execution_unit_sp,
exe_ctx, call_static_initializer, options, execution_errors);
if (results != eExpressionCompleted) {
- err = Status::FromErrorStringWithFormat(
- "couldn't run static initializer: %s",
- execution_errors.GetString().c_str());
+ err = Status::FromError(execution_errors.GetAsError(
+ lldb::eExpressionSetupError, "couldn't run static initializer:"));
return err;
}
}
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 872f6304f91baa..b3c81af24893d0 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -328,18 +328,20 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
}
if (!parse_success) {
- std::string msg;
- {
- llvm::raw_string_ostream os(msg);
- if (!diagnostic_manager.Diagnostics().empty())
- os << diagnostic_manager.GetString();
- else
- os << "expression failed to parse (no further compiler diagnostics)";
- if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
- !fixed_expression->empty())
- os << "\nfixed expression suggested:\n " << *fixed_expression;
+ if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
+ !fixed_expression->empty()) {
+ std::string fixit =
+ "fixed expression suggested:\n " + *fixed_expression;
+ diagnostic_manager.AddDiagnostic(fixit, lldb::eSeverityInfo,
+ eDiagnosticOriginLLDB);
}
- error = Status::FromExpressionError(execution_results, msg);
+ if (diagnostic_manager.Diagnostics().empty())
+ error = Status::FromError(llvm::make_error<ExpressionError>(
+ execution_results,
+ "expression failed to parse (no further compiler diagnostics)"));
+ else
+ error =
+ Status::FromError(diagnostic_manager.GetAsError(execution_results));
}
}
@@ -351,18 +353,18 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
LLDB_LOG(log, "== [UserExpression::Evaluate] Expression may not run, but "
"is not constant ==");
- if (!diagnostic_manager.Diagnostics().size())
- error = Status::FromExpressionError(
+ if (diagnostic_manager.Diagnostics().empty())
+ error = Status::FromError(llvm::make_error<ExpressionError>(
lldb::eExpressionSetupError,
- "expression needed to run but couldn't");
+ "expression needed to run but couldn't"));
} else if (execution_policy == eExecutionPolicyTopLevel) {
error = Status(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
return lldb::eExpressionCompleted;
} else {
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
- error = Status::FromExpressionError(
+ error = Status::FromError(llvm::make_error<ExpressionError>(
lldb::eExpressionInterrupted,
- "expression interrupted by callback before execution");
+ "expression interrupted by callback before execution"));
result_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(), std::move(error));
return lldb::eExpressionInterrupted;
@@ -380,12 +382,13 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
LLDB_LOG(log, "== [UserExpression::Evaluate] Execution completed "
"abnormally ==");
- if (!diagnostic_manager.Diagnostics().size())
- error = Status::FromExpressionError(
- execution_results, "expression failed to execute, unknown error");
+ if (diagnostic_manager.Diagnostics().empty())
+ error = Status::FromError(llvm::make_error<ExpressionError>(
+ execution_results,
+ "expression failed to execute, unknown error"));
else
- error = Status::FromExpressionError(execution_results,
- diagnostic_manager.GetString());
+ error = Status::FromError(
+ diagnostic_manager.GetAsError(execution_results));
} else {
if (expr_result) {
result_valobj_sp = expr_result->GetValueObject();
@@ -407,9 +410,9 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
}
if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
- error = Status::FromExpressionError(
+ error = Status::FromError(llvm::make_error<ExpressionError>(
lldb::eExpressionInterrupted,
- "expression interrupted by callback after complete");
+ "expression interrupted by callback after complete"));
return lldb::eExpressionInterrupted;
}
diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp
index 55ebfb8ef342e8..97c226ae1c5f95 100644
--- a/lldb/source/Expression/UtilityFunction.cpp
+++ b/lldb/source/Expression/UtilityFunction.cpp
@@ -83,19 +83,19 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
m_caller_up.reset(process_sp->GetTarget().GetFunctionCallerForLanguage(
Language().AsLanguageType(), return_type, impl_code_address,
arg_value_list, name.c_str(), error));
- if (error.Fail()) {
-
+ if (error.Fail())
return nullptr;
- }
+
if (m_caller_up) {
DiagnosticManager diagnostics;
unsigned num_errors =
m_caller_up->CompileFunction(thread_to_use_sp, diagnostics);
if (num_errors) {
- error = Status::FromErrorStringWithFormat(
- "Error compiling %s caller function: \"%s\".",
- m_function_name.c_str(), diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ lldb::eExpressionParseError,
+ "Error compiling " + m_function_name + " caller function:"));
+
m_caller_up.reset();
return nullptr;
}
@@ -104,9 +104,9 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
ExecutionContext exe_ctx(process_sp);
if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostics)) {
- error = Status::FromErrorStringWithFormat(
- "Error inserting caller function for %s: \"%s\".",
- m_function_name.c_str(), diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ lldb::eExpressionSetupError,
+ "Error inserting " + m_function_name + " caller function:"));
m_caller_up.reset();
return nullptr;
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
index 21abd71cc34eeb..c473df808ee8d0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
@@ -29,9 +29,8 @@ class ClangDiagnostic : public Diagnostic {
return diag->getKind() == eDiagnosticOriginClang;
}
- ClangDiagnostic(llvm::StringRef message, lldb::Severity severity,
- uint32_t compiler_id)
- : Diagnostic(message, severity, eDiagnosticOriginClang, compiler_id) {}
+ ClangDiagnostic(DiagnosticDetail detail, uint32_t compiler_id)
+ : Diagnostic(eDiagnosticOriginClang, compiler_id, detail) {}
~ClangDiagnostic() override = default;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 2fe3c0460aa7f8..92470054507e66 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -26,6 +26,7 @@
#include "clang/Frontend/FrontendActions.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Frontend/TextDiagnostic.h"
#include "clang/Frontend/TextDiagnosticBuffer.h"
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Lex/Preprocessor.h"
@@ -161,7 +162,8 @@ static void AddAllFixIts(ClangDiagnostic *diag, const clang::Diagnostic &Info) {
class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
public:
- ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) {
+ ClangDiagnosticManagerAdapter(DiagnosticOptions &opts, StringRef filename)
+ : m_filename(filename) {
DiagnosticOptions *options = new DiagnosticOptions(opts);
options->ShowPresumedLoc = true;
options->ShowLevel = false;
@@ -212,20 +214,20 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
m_passthrough->HandleDiagnostic(DiagLevel, Info);
m_os->flush();
- lldb::Severity severity;
+ DiagnosticDetail detail;
bool make_new_diagnostic = true;
switch (DiagLevel) {
case DiagnosticsEngine::Level::Fatal:
case DiagnosticsEngine::Level::Error:
- severity = lldb::eSeverityError;
+ detail.severity = lldb::eSeverityError;
break;
case DiagnosticsEngine::Level::Warning:
- severity = lldb::eSeverityWarning;
+ detail.severity = lldb::eSeverityWarning;
break;
case DiagnosticsEngine::Level::Remark:
case DiagnosticsEngine::Level::Ignored:
- severity = lldb::eSeverityInfo;
+ detail.severity = lldb::eSeverityInfo;
break;
case DiagnosticsEngine::Level::Note:
m_manager->AppendMessageToDiagnostic(m_output);
@@ -254,14 +256,46 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
std::string stripped_output =
std::string(llvm::StringRef(m_output).trim());
- auto new_diagnostic = std::make_unique<ClangDiagnostic>(
- stripped_output, severity, Info.getID());
+ // Translate the source location.
+ if (Info.hasSourceManager()) {
+ DiagnosticDetail::SourceLocation loc;
+ clang::SourceManager &sm = Info.getSourceManager();
+ const clang::SourceLocation sloc = Info.getLocation();
+ if (sloc.isValid()) {
+ const clang::FullSourceLoc fsloc(sloc, sm);
+ clang::PresumedLoc PLoc = fsloc.getPresumedLoc(true);
+ StringRef filename =
+ PLoc.isValid() ? PLoc.getFilename() : StringRef{};
+ loc.file = FileSpec(filename);
+ loc.line = fsloc.getSpellingLineNumber();
+ loc.column = fsloc.getSpellingColumnNumber();
+ loc.in_user_input = filename == m_filename;
+
+ // Find the range of the primary location.
+ for (const auto &range : Info.getRanges()) {
+ if (range.getBegin() == sloc) {
+ // FIXME: This is probably not handling wide characters correctly.
+ unsigned end_col = sm.getSpellingColumnNumber(range.getEnd());
+ if (end_col > loc.column)
+ loc.length = end_col - loc.column;
+ break;
+ }
+ }
+ detail.source_location = loc;
+ }
+ }
+ llvm::SmallString<0> msg;
+ Info.FormatDiagnostic(msg);
+ detail.message = msg.str();
+ detail.rendered = stripped_output;
+ auto new_diagnostic =
+ std::make_unique<ClangDiagnostic>(detail, Info.getID());
// Don't store away warning fixits, since the compiler doesn't have
// enough context in an expression for the warning to be useful.
// FIXME: Should we try to filter out FixIts that apply to our generated
// code, and not the user's expression?
- if (severity == lldb::eSeverityError)
+ if (detail.severity == lldb::eSeverityError)
AddAllFixIts(new_diagnostic.get(), Info);
m_manager->AddDiagnostic(std::move(new_diagnostic));
@@ -281,6 +315,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
std::shared_ptr<llvm::raw_string_ostream> m_os;
/// Output string filled by m_os.
std::string m_output;
+ StringRef m_filename;
};
/// Returns true if the SDK for the specified triple supports
@@ -711,8 +746,8 @@ ClangExpressionParser::ClangExpressionParser(
// 4. Set language options.
SetupLangOpts(*m_compiler, *exe_scope, expr);
- if (auto *clang_expr = dyn_cast<ClangUserExpression>(&m_expr);
- clang_expr && clang_expr->DidImportCxxModules()) {
+ auto *clang_expr = dyn_cast<ClangUserExpression>(&m_expr);
+ if (clang_expr && clang_expr->DidImportCxxModules()) {
LLDB_LOG(log, "Adding lang options for importing C++ modules");
SetupImportStdModuleLangOpts(*m_compiler, *target_sp);
SetupModuleHeaderPaths(m_compiler.get(), m_include_directories, target_sp);
@@ -739,9 +774,9 @@ ClangExpressionParser::ClangExpressionParser(
m_compiler->getLangOpts());
// 5. Set up the diagnostic buffer for reporting errors
-
auto diag_mgr = new ClangDiagnosticManagerAdapter(
- m_compiler->getDiagnostics().getDiagnosticOptions());
+ m_compiler->getDiagnostics().getDiagnosticOptions(),
+ clang_expr ? clang_expr->GetFilename() : StringRef());
m_compiler->getDiagnostics().setClient(diag_mgr);
// 6. Set up the source management objects inside the compiler
@@ -1503,13 +1538,9 @@ lldb_private::Status ClangExpressionParser::DoPrepareForExecution(
new ClangDynamicCheckerFunctions();
DiagnosticManager install_diags;
- if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx)) {
- std::string ErrMsg = "couldn't install checkers: " + toString(std::move(Err));
- if (install_diags.Diagnostics().size())
- ErrMsg = ErrMsg + "\n" + install_diags.GetString().c_str();
- err = Status(ErrMsg);
- return err;
- }
+ if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx))
+ return Status::FromError(install_diags.GetAsError(
+ lldb::eExpressionSetupError, "couldn't install checkers:"));
process->SetDynamicCheckers(dynamic_checkers);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
index 09604feea5dec4..7c0c6a0147e2a0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -177,6 +177,8 @@ class ClangUserExpression : public LLVMUserExpression {
/// Returns true iff this expression is using any imported C++ modules.
bool DidImportCxxModules() const { return !m_imported_cpp_modules.empty(); }
+ llvm::StringRef GetFilename() const { return m_filename; }
+
private:
/// Populate m_in_cplusplus_method and m_in_objectivec_method based on the
/// environment.
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 31315e46ca168d..e9830c9f8722b2 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -863,9 +863,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
func_args_addr,
arguments,
diagnostics)) {
- error = Status::FromErrorStringWithFormat(
- "dlopen error: could not write function arguments: %s",
- diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ lldb::eExpressionSetupError,
+ "dlopen error: could not write function arguments:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
@@ -906,9 +906,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
ExpressionResults results = do_dlopen_function->ExecuteFunction(
exe_ctx, &func_args_addr, options, diagnostics, return_value);
if (results != eExpressionCompleted) {
- error = Status::FromErrorStringWithFormat(
- "dlopen error: failed executing dlopen wrapper function: %s",
- diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ lldb::eExpressionSetupError,
+ "dlopen error: failed executing dlopen wrapper function:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
index 7352d6f33f2174..3936b8367fb83b 100644
--- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -341,9 +341,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
diagnostics.Clear();
if (!invocation->WriteFunctionArguments(context, injected_parameters,
parameters, diagnostics)) {
- error = Status::FromErrorStringWithFormat(
- "LoadLibrary error: unable to write function parameters: %s",
- diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ eExpressionSetupError,
+ "LoadLibrary error: unable to write function parameters:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
@@ -384,9 +384,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
invocation->ExecuteFunction(context, &injected_parameters, options,
diagnostics, value);
if (result != eExpressionCompleted) {
- error = Status::FromErrorStringWithFormat(
- "LoadLibrary error: failed to execute LoadLibrary helper: %s",
- diagnostics.GetString().c_str());
+ error = Status::FromError(diagnostics.GetAsError(
+ eExpressionSetupError,
+ "LoadLibrary error: failed to execute LoadLibrary helper:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 29e9efb83efeb6..7e45264ec3d118 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2680,7 +2680,8 @@ Target::CreateUtilityFunction(std::string expression, std::string name,
DiagnosticManager diagnostics;
if (!utility_fn->Install(diagnostics, exe_ctx))
- return llvm::createStringError(diagnostics.GetString());
+ return diagnostics.GetAsError(lldb::eExpressionSetupError,
+ "Could not install utility function:");
return std::move(utility_fn);
}
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 7f73962c7fc9aa..fd5c5537d8488a 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -43,7 +43,7 @@ char CloneableError::ID;
char CloneableECError::ID;
char MachKernelError::ID;
char Win32Error::ID;
-char ExpressionError::ID;
+char ExpressionErrorBase::ID;
namespace {
/// A std::error_code category for eErrorTypeGeneric.
@@ -55,21 +55,6 @@ LLDBGenericCategory &lldb_generic_category() {
static LLDBGenericCategory g_generic_category;
return g_generic_category;
}
-
-/// A std::error_code category for eErrorTypeExpression.
-class ExpressionCategory : public std::error_category {
- const char *name() const noexcept override {
- return "LLDBExpressionCategory";
- }
- std::string message(int __ev) const override {
- return ExpressionResultAsCString(
- static_cast<lldb::ExpressionResults>(__ev));
- };
-};
-ExpressionCategory &expression_category() {
- static ExpressionCategory g_expression_category;
- return g_expression_category;
-}
} // namespace
Status::Status() : m_error(llvm::Error::success()) {}
@@ -132,12 +117,6 @@ Status Status::FromErrorStringWithFormat(const char *format, ...) {
return Status(string);
}
-Status Status::FromExpressionError(lldb::ExpressionResults result,
- std::string msg) {
- return Status(llvm::make_error<ExpressionError>(
- std::error_code(result, expression_category()), msg));
-}
-
/// Creates a deep copy of all known errors and converts all other
/// errors to a new llvm::StringError.
static llvm::Error CloneError(const llvm::Error &error) {
@@ -211,10 +190,6 @@ std::unique_ptr<CloneableError> Win32Error::Clone() const {
return std::make_unique<Win32Error>(convertToErrorCode());
}
-std::unique_ptr<CloneableError> ExpressionError::Clone() const {
- return std::make_unique<ExpressionError>(convertToErrorCode(), message());
-}
-
// Get the error value as a NULL C string. The error string will be fetched and
// cached on demand. The cached error string value will remain until the error
// value is changed or cleared.
@@ -268,7 +243,7 @@ ErrorType Status::GetType() const {
result = eErrorTypeMachKernel;
else if (error.isA<Win32Error>())
result = eErrorTypeWin32;
- else if (error.isA<ExpressionError>())
+ else if (error.isA<ExpressionErrorBase>())
result = eErrorTypeExpression;
else if (error.convertToErrorCode().category() == std::generic_category())
result = eErrorTypePOSIX;
diff --git a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
index 620b6e44fc852a..36e302be2525b5 100644
--- a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
+++ b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
@@ -21,7 +21,7 @@ def test(self):
"expr @import LLDBTestModule",
error=True,
substrs=[
- "module.h:4:1: use of undeclared identifier 'syntax_error_for_lldb_to_find'",
+ "module.h:4:1: error: use of undeclared identifier 'syntax_error_for_lldb_to_find'",
"syntax_error_for_lldb_to_find // comment that tests source printing",
"could not build module 'LLDBTestModule'",
],
diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
index 05fe7c164d6818..4608b779f79a96 100644
--- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -19,8 +19,8 @@ class FixItDiag : public Diagnostic {
public:
FixItDiag(llvm::StringRef msg, bool has_fixits)
- : Diagnostic(msg, lldb::eSeverityError,
- DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id),
+ : Diagnostic(DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id,
+ DiagnosticDetail{{}, lldb::eSeverityError, msg.str(), {}}),
m_has_fixits(has_fixits) {}
bool HasFixIts() const override { return m_has_fixits; }
};
@@ -30,8 +30,8 @@ namespace {
class TextDiag : public Diagnostic {
public:
TextDiag(llvm::StringRef msg, lldb::Severity severity)
- : Diagnostic(msg, severity, DiagnosticOrigin::eDiagnosticOriginLLDB,
- custom_diag_id) {}
+ : Diagnostic(DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id,
+ DiagnosticDetail{{}, severity, msg.str(), msg.str()}) {}
};
} // namespace
@@ -42,8 +42,8 @@ TEST(DiagnosticManagerTest, AddDiagnostic) {
std::string msg = "foo bar has happened";
lldb::Severity severity = lldb::eSeverityError;
DiagnosticOrigin origin = DiagnosticOrigin::eDiagnosticOriginLLDB;
- auto diag =
- std::make_unique<Diagnostic>(msg, severity, origin, custom_diag_id);
+ auto diag = std::make_unique<Diagnostic>(
+ origin, custom_diag_id, DiagnosticDetail{{}, severity, msg, {}});
mgr.AddDiagnostic(std::move(diag));
EXPECT_EQ(1U, mgr.Diagnostics().size());
const Diagnostic *got = mgr.Diagnostics().front().get();
@@ -197,3 +197,13 @@ TEST(DiagnosticManagerTest, FixedExpression) {
mgr.SetFixedExpression("bar");
EXPECT_EQ("bar", mgr.GetFixedExpression());
}
+
+TEST(DiagnosticManagerTest, StatusConversion) {
+ DiagnosticManager mgr;
+ mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError));
+ mgr.AddDiagnostic(std::make_unique<TextDiag>("def", lldb::eSeverityWarning));
+ Status status =
+ Status::FromError(mgr.GetAsError(lldb::eExpressionParseError));
+ EXPECT_EQ(std::string("error: abc\nwarning: def\n"),
+ std::string(status.AsCString()));
+}
>From a51d8a684ab1eade42ad8716bdf15e2e0216249a Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Wed, 28 Aug 2024 16:19:21 -0700
Subject: [PATCH 2/2] [lldb] Inline expression evaluator error visualization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938
Before:
```
$ lldb -o "expr a+b"
(lldb) expr a+b
error: <user expression 0>:1:1: use of undeclared identifier 'a'
a+b
^
error: <user expression 0>:1:3: use of undeclared identifier 'b'
a+b
^
```
After:
```
(lldb) expr a+b
^ ^
│ ╰─ error: use of undeclared identifier 'b'
╰─ error: use of undeclared identifier 'a'
```
This eliminates the confusing `<user expression 0>:1:3` source
location and avoids echoing the expression to the console again, which
results in a cleaner presentation that makes it easier to grasp what's
going on.
---
lldb/include/lldb/API/SBDebugger.h | 2 +
lldb/include/lldb/Core/Debugger.h | 4 +
.../lldb/Expression/DiagnosticManager.h | 3 +-
lldb/include/lldb/Interpreter/CommandObject.h | 8 +
lldb/source/API/SBDebugger.cpp | 6 +
.../Commands/CommandObjectExpression.cpp | 154 ++++++++++++++++--
lldb/source/Core/CoreProperties.td | 4 +
lldb/source/Core/Debugger.cpp | 13 +-
lldb/source/Expression/DiagnosticManager.cpp | 2 +-
.../source/Interpreter/CommandInterpreter.cpp | 4 +-
.../Clang/ClangExpressionParser.cpp | 27 +--
.../ctf/CommandObjectThreadTraceExportCTF.h | 2 +-
lldb/source/Utility/Status.cpp | 2 -
.../diagnostics/TestExprDiagnostics.py | 51 ++++++
.../TestPersistentVariables.py | 4 +-
.../TestStaticInitializers.py | 2 +-
.../TestTemplateFunctions.py | 3 +-
.../test/API/lang/mixed/TestMixedLanguages.py | 2 +-
lldb/test/Shell/Expr/TestObjCIDCast.test | 2 +-
.../test/Shell/Expr/TestObjCInCXXContext.test | 2 +-
.../NativePDB/incomplete-tag-type.cpp | 4 +-
lldb/tools/driver/Driver.cpp | 1 +
.../Expression/DiagnosticManagerTest.cpp | 33 ++--
lldb/unittests/Interpreter/CMakeLists.txt | 1 +
.../TestCommandObjectExpression.cpp | 34 ++++
25 files changed, 316 insertions(+), 54 deletions(-)
create mode 100644 lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 84ea9c0f772e16..6afa1c932ab050 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -304,6 +304,8 @@ class LLDB_API SBDebugger {
bool GetUseColor() const;
+ bool SetShowInlineDiagnostics(bool);
+
bool SetUseSourceCache(bool use_source_cache);
bool GetUseSourceCache() const;
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index a72c2596cc2c5e..1d5f2fcc20626c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -364,6 +364,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
const std::string &GetInstanceName() { return m_instance_name; }
+ bool GetShowInlineDiagnostics() const;
+
+ bool SetShowInlineDiagnostics(bool);
+
bool LoadPlugin(const FileSpec &spec, Status &error);
void RunIOHandlers();
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index 3e363ee5c0139c..3bb963ce8d01c7 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -39,6 +39,7 @@ struct DiagnosticDetail {
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.
@@ -64,7 +65,7 @@ class ExpressionError
ExpressionError(lldb::ExpressionResults result, std::string msg,
std::vector<DiagnosticDetail> details = {});
std::string message() const override;
- llvm::ArrayRef<DiagnosticDetail> GetDetail() const { return m_details; }
+ llvm::ArrayRef<DiagnosticDetail> GetDetails() const { 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/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h
index 20c4769af90338..c5167e5e0ecb6a 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -340,6 +340,13 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
return false;
}
+ /// Set the command input as it appeared in the terminal. This
+ /// is used to have errors refer directly to the original command.
+ void SetOriginalCommandString(std::string s) { m_original_command = s; }
+
+ /// \param offset_in_command is on what column \c args_string
+ /// appears, if applicable. This enables diagnostics that refer back
+ /// to the user input.
virtual void Execute(const char *args_string,
CommandReturnObject &result) = 0;
@@ -404,6 +411,7 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
std::string m_cmd_help_short;
std::string m_cmd_help_long;
std::string m_cmd_syntax;
+ std::string m_original_command;
Flags m_flags;
std::vector<CommandArgumentEntry> m_arguments;
lldb::CommandOverrideCallback m_deprecated_command_override_callback;
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 6b72994fc96afb..47931f1c16f9a3 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1483,6 +1483,12 @@ bool SBDebugger::GetUseColor() const {
return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
}
+bool SBDebugger::SetShowInlineDiagnostics(bool value) {
+ LLDB_INSTRUMENT_VA(this, value);
+
+ return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false);
+}
+
bool SBDebugger::SetUseSourceCache(bool value) {
LLDB_INSTRUMENT_VA(this, value);
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 771194638e1b65..49f4421fdb54f9 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -10,6 +10,7 @@
#include "CommandObjectExpression.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"
@@ -398,6 +399,122 @@ CanBeUsedForElementCountPrinting(ValueObject &valobj) {
return Status();
}
+static llvm::raw_ostream &PrintSeverity(Stream &stream,
+ lldb::Severity severity) {
+ llvm::HighlightColor color;
+ llvm::StringRef text;
+ switch (severity) {
+ case eSeverityError:
+ color = llvm::HighlightColor::Error;
+ text = "error: ";
+ break;
+ case eSeverityWarning:
+ color = llvm::HighlightColor::Warning;
+ text = "warning: ";
+ break;
+ case eSeverityInfo:
+ color = llvm::HighlightColor::Remark;
+ text = "note: ";
+ break;
+ }
+ return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable)
+ << text;
+}
+
+namespace lldb_private {
+// Public for unittesting.
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional<uint16_t> offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef<DiagnosticDetail> details) {
+ if (details.empty())
+ return;
+
+ if (!offset_in_command) {
+ for (const DiagnosticDetail &detail : details) {
+ PrintSeverity(stream, detail.severity);
+ stream << detail.rendered << '\n';
+ }
+ return;
+ }
+
+ // Print a line with caret indicator(s) below the lldb prompt + command.
+ const size_t padding = *offset_in_command;
+ stream << std::string(padding, ' ');
+
+ size_t offset = 1;
+ std::vector<DiagnosticDetail> remaining_details, other_details,
+ hidden_details;
+ for (const DiagnosticDetail &detail : details) {
+ if (!show_inline || !detail.source_location) {
+ other_details.push_back(detail);
+ continue;
+ }
+ if (detail.source_location->hidden) {
+ hidden_details.push_back(detail);
+ continue;
+ }
+ if (!detail.source_location->in_user_input) {
+ other_details.push_back(detail);
+ continue;
+ }
+
+ auto &loc = *detail.source_location;
+ 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, '~');
+ offset = loc.column + 1;
+ }
+ stream << '\n';
+
+ // Work through each detail in reverse order using the vector/stack.
+ bool did_print = false;
+ for (auto detail = remaining_details.rbegin();
+ detail != remaining_details.rend();
+ ++detail, remaining_details.pop_back()) {
+ // Get the information to print this detail and remove it from the stack.
+ // Print all the lines for all the other messages first.
+ stream << std::string(padding, ' ');
+ size_t offset = 1;
+ for (auto &remaining_detail :
+ llvm::ArrayRef(remaining_details).drop_back(1)) {
+ uint16_t column = remaining_detail.source_location->column;
+ stream << std::string(column - offset, ' ') << "│";
+ 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, ' ') << "╰─ ";
+
+ // Print a colorized string based on the message's severity type.
+ PrintSeverity(stream, detail->severity);
+
+ // Finally, print the message and start a new line.
+ stream << detail->message << '\n';
+ did_print = true;
+ }
+
+ // Print the non-located details.
+ for (const DiagnosticDetail &detail : other_details) {
+ PrintSeverity(stream, detail.severity);
+ stream << detail.rendered << '\n';
+ did_print = true;
+ }
+
+ // Print the hidden details as a last resort.
+ if (!did_print)
+ for (const DiagnosticDetail &detail : hidden_details) {
+ PrintSeverity(stream, detail.severity);
+ stream << detail.rendered << '\n';
+ }
+}
+} // namespace lldb_private
+
bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
Stream &output_stream,
Stream &error_stream,
@@ -486,19 +603,34 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
result.SetStatus(eReturnStatusSuccessFinishResult);
} else {
- const char *error_cstr = result_valobj_sp->GetError().AsCString();
- if (error_cstr && error_cstr[0]) {
- const size_t error_cstr_len = strlen(error_cstr);
- const bool ends_with_newline = error_cstr[error_cstr_len - 1] == '\n';
- if (strstr(error_cstr, "error:") != error_cstr)
- error_stream.PutCString("error: ");
- error_stream.Write(error_cstr, error_cstr_len);
- if (!ends_with_newline)
- error_stream.EOL();
+ // Retrieve the diagnostics.
+ std::vector<DiagnosticDetail> details;
+ llvm::consumeError(llvm::handleErrors(
+ result_valobj_sp->GetError().ToError(),
+ [&](ExpressionError &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);
+ if (nchar != std::string::npos)
+ expr_pos = nchar + GetDebugger().GetPrompt().size();
+
+ if (!details.empty()) {
+ bool show_inline =
+ GetDebugger().GetShowInlineDiagnostics() && !expr.contains('\n');
+ RenderDiagnosticDetails(error_stream, expr_pos, show_inline, details);
} else {
- error_stream.PutCString("error: unknown error\n");
+ const char *error_cstr = result_valobj_sp->GetError().AsCString();
+ llvm::StringRef error(error_cstr);
+ if (!error.empty()) {
+ if (!error.starts_with("error:"))
+ error_stream << "error: ";
+ error_stream << error;
+ if (!error.ends_with('\n'))
+ error_stream.EOL();
+ } else {
+ error_stream << "error: unknown error\n";
+ }
}
-
result.SetStatus(eReturnStatusFailed);
}
}
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index a6cb951187a040..e11aad2660b461 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -225,4 +225,8 @@ let Definition = "debugger" in {
DefaultEnumValue<"eDWIMPrintVerbosityNone">,
EnumValues<"OptionEnumValues(g_dwim_print_verbosities)">,
Desc<"The verbosity level used by dwim-print.">;
+ def ShowInlineDiagnostics: Property<"show-inline-diagnostics", "Boolean">,
+ Global,
+ DefaultFalse,
+ Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">;
}
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 9bdc5a3949751d..e6b9eedd89b4e3 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -592,7 +592,18 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const {
const uint32_t idx = ePropertyDWIMPrintVerbosity;
return GetPropertyAtIndexAs<lldb::DWIMPrintVerbosity>(
idx, static_cast<lldb::DWIMPrintVerbosity>(
- g_debugger_properties[idx].default_uint_value));
+ g_debugger_properties[idx].default_uint_value != 0));
+}
+
+bool Debugger::GetShowInlineDiagnostics() const {
+ const uint32_t idx = ePropertyShowInlineDiagnostics;
+ return GetPropertyAtIndexAs<bool>(
+ idx, g_debugger_properties[idx].default_uint_value);
+}
+
+bool Debugger::SetShowInlineDiagnostics(bool b) {
+ const uint32_t idx = ePropertyShowInlineDiagnostics;
+ return SetPropertyAtIndex(idx, b);
}
#pragma mark Debugger
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index 3731a200c5e2f7..7c67a0ce4aa026 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -37,7 +37,7 @@ ExpressionError::ExpressionError(lldb::ExpressionResults result,
: ErrorInfo(std::error_code(result, expression_category())), m_message(msg),
m_details(details) {}
-static const char *StringForSeverity(lldb::Severity severity) {
+static llvm::StringRef StringForSeverity(lldb::Severity severity) {
switch (severity) {
// this should be exhaustive
case lldb::eSeverityError:
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index acd592c3bd2dbc..d17aa6fec1f00e 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1887,7 +1887,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
CommandReturnObject &result,
bool force_repeat_command) {
std::string command_string(command_line);
- std::string original_command_string(command_line);
+ std::string original_command_string(command_string);
+ std::string real_original_command_string(command_string);
Log *log = GetLog(LLDBLog::Commands);
llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
@@ -2076,6 +2077,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
}
ElapsedTime elapsed(execute_time);
+ cmd_obj->SetOriginalCommandString(real_original_command_string);
cmd_obj->Execute(remainder.c_str(), result);
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 92470054507e66..39eb86d4f0947a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -176,15 +176,22 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
m_manager = manager;
}
- /// Returns the last ClangDiagnostic message that the DiagnosticManager
- /// received or a nullptr if the DiagnosticMangager hasn't seen any
- /// Clang diagnostics yet.
+ /// Returns the last error ClangDiagnostic message that the
+ /// DiagnosticManager received or a nullptr.
ClangDiagnostic *MaybeGetLastClangDiag() const {
if (m_manager->Diagnostics().empty())
return nullptr;
- lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
- ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag);
- return clang_diag;
+ auto &diags = m_manager->Diagnostics();
+ for (auto it = diags.rbegin(); it != diags.rend(); it++) {
+ lldb_private::Diagnostic *diag = it->get();
+ if (ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag)) {
+ if (clang_diag->GetSeverity() == lldb::eSeverityWarning)
+ return nullptr;
+ if (clang_diag->GetSeverity() == lldb::eSeverityError)
+ return clang_diag;
+ }
+ }
+ return nullptr;
}
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
@@ -215,8 +222,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
m_os->flush();
DiagnosticDetail detail;
- bool make_new_diagnostic = true;
-
switch (DiagLevel) {
case DiagnosticsEngine::Level::Fatal:
case DiagnosticsEngine::Level::Error:
@@ -230,9 +235,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
detail.severity = lldb::eSeverityInfo;
break;
case DiagnosticsEngine::Level::Note:
- m_manager->AppendMessageToDiagnostic(m_output);
- make_new_diagnostic = false;
-
// 'note:' diagnostics for errors and warnings can also contain Fix-Its.
// We add these Fix-Its to the last error diagnostic to make sure
// that we later have all Fix-Its related to an 'error' diagnostic when
@@ -250,7 +252,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
AddAllFixIts(clang_diag, Info);
break;
}
- if (make_new_diagnostic) {
// ClangDiagnostic messages are expected to have no whitespace/newlines
// around them.
std::string stripped_output =
@@ -270,6 +271,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
loc.line = fsloc.getSpellingLineNumber();
loc.column = fsloc.getSpellingColumnNumber();
loc.in_user_input = filename == m_filename;
+ loc.hidden = filename.starts_with("<lldb wrapper ");
// Find the range of the primary location.
for (const auto &range : Info.getRanges()) {
@@ -299,7 +301,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
AddAllFixIts(new_diagnostic.get(), Info);
m_manager->AddDiagnostic(std::move(new_diagnostic));
- }
}
void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override {
diff --git a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
index 1a034e87cfb65b..06834edf14ea16 100644
--- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
+++ b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
@@ -48,7 +48,7 @@ class CommandObjectThreadTraceExportCTF : public CommandObjectParsed {
Options *GetOptions() override { return &m_options; }
protected:
- void DoExecute(Args &command, CommandReturnObject &result) override;
+ void DoExecute(Args &args, CommandReturnObject &result) override;
CommandOptions m_options;
};
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index fd5c5537d8488a..1c85103def3fbb 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -250,8 +250,6 @@ ErrorType Status::GetType() const {
else if (error.convertToErrorCode().category() == lldb_generic_category() ||
error.convertToErrorCode() == llvm::inconvertibleErrorCode())
result = eErrorTypeGeneric;
- else
- result = eErrorTypeInvalid;
});
return result;
}
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index ddc1c3598480cf..e09b4f6099476a 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -183,3 +183,54 @@ def test_source_locations_from_objc_modules(self):
# The NSLog definition source line should be printed. Return value and
# the first argument are probably stable enough that this test can check for them.
self.assertIn("void NSLog(NSString *format", value.GetError().GetCString())
+
+ def test_command_expr_formatting(self):
+ """Test that the source and caret positions LLDB prints are correct"""
+ self.build()
+
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "// Break here", self.main_source_spec
+ )
+ frame = thread.GetFrameAtIndex(0)
+ self.expect('settings set show-inline-diagnostics true')
+
+ def check(input_ref):
+ self.expect(input_ref[0], error=True, substrs=input_ref[1:])
+
+ check(
+ [
+ "expression -- a+b",
+ " ^ ^",
+ " │ ╰─ error: use of undeclared identifier 'b'",
+ " ╰─ error: use of undeclared identifier 'a'",
+ ]
+ )
+
+ check(
+ [
+ "expr -- a",
+ " ^",
+ " ╰─ error: use of undeclared identifier 'a'",
+ ]
+ )
+ check(
+ [
+ "expr -i 0 -o 0 -- a",
+ " ^",
+ " ╰─ error: use of undeclared identifier 'a'",
+ ]
+ )
+
+ self.expect(
+ "expression --top-level -- template<typename T> T FOO(T x) { return x/2;}"
+ )
+ check(
+ [
+ 'expression -- FOO("")',
+ " ^",
+ " ╰─ note: in instantiation of function template specialization 'FOO<const char *>' requested here",
+ "error: <user expression",
+ "invalid operands to binary expression",
+ ]
+ )
+ check(["expression --\na\n+\nb", "error: <user", "a", "error: <user", "b"])
diff --git a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
index 6a7995ff2a837e..d0d9358589a719 100644
--- a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
+++ b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
@@ -56,7 +56,7 @@ def test_persistent_variables(self):
self.expect(
"expr int $i = 123",
error=True,
- substrs=["error: redefinition of persistent variable '$i'"],
+ substrs=["redefinition of persistent variable '$i'"],
)
self.expect_expr("$i", result_type="int", result_value="5")
@@ -65,7 +65,7 @@ def test_persistent_variables(self):
self.expect(
"expr long $i = 123",
error=True,
- substrs=["error: redefinition of persistent variable '$i'"],
+ substrs=["redefinition of persistent variable '$i'"],
)
self.expect_expr("$i", result_type="int", result_value="5")
diff --git a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
index ea3aa6a4608c41..c734033bd6c028 100644
--- a/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
+++ b/lldb/test/API/commands/expression/static-initializers/TestStaticInitializers.py
@@ -35,7 +35,7 @@ def test_failing_init(self):
self.expect(
"expr -p -- struct Foo2 { Foo2() { do_abort(); } }; Foo2 f;",
error=True,
- substrs=["error: couldn't run static initializer:"],
+ substrs=["couldn't run static initializer:"],
)
def test_without_process(self):
diff --git a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
index 71df90e6a6d161..3be93dedfd11df 100644
--- a/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
+++ b/lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
@@ -21,7 +21,8 @@ def do_test_template_function(self, add_cast):
"expr b1 <=> b2",
error=True,
substrs=[
- "warning: <user expression 0>:1:4: '<=>' is a single token in C++20; add a space to avoid a change in behavior"
+ "warning:",
+ "'<=>' is a single token in C++20; add a space to avoid a change in behavior",
],
)
diff --git a/lldb/test/API/lang/mixed/TestMixedLanguages.py b/lldb/test/API/lang/mixed/TestMixedLanguages.py
index 8b73254cce4a93..1637d59a5edcba 100644
--- a/lldb/test/API/lang/mixed/TestMixedLanguages.py
+++ b/lldb/test/API/lang/mixed/TestMixedLanguages.py
@@ -40,7 +40,7 @@ def cleanup():
self.runCmd("run")
self.expect("thread backtrace", substrs=["`main", "lang=c"])
# Make sure evaluation of C++11 fails.
- self.expect("expr foo != nullptr", error=True, startstr="error")
+ self.expect("expr foo != nullptr", error=True, substrs=["error"])
# Run to BP at foo (in foo.cpp) and test that the language is C++.
self.runCmd("breakpoint set -n foo")
diff --git a/lldb/test/Shell/Expr/TestObjCIDCast.test b/lldb/test/Shell/Expr/TestObjCIDCast.test
index 0611171da09e2e..19ca404643c1d9 100644
--- a/lldb/test/Shell/Expr/TestObjCIDCast.test
+++ b/lldb/test/Shell/Expr/TestObjCIDCast.test
@@ -6,4 +6,4 @@
// RUN: 2>&1 | FileCheck %s
// CHECK: (lldb) expression --language objc -- *(id)0x1
-// CHECK: error: Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory
+// CHECK: error:{{.*}}Couldn't apply expression side effects : Couldn't dematerialize a result variable: couldn't read its memory
diff --git a/lldb/test/Shell/Expr/TestObjCInCXXContext.test b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
index 8537799bdeb674..f8cad5b58a1e53 100644
--- a/lldb/test/Shell/Expr/TestObjCInCXXContext.test
+++ b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
@@ -18,4 +18,4 @@
// CHECK-NEXT: (NSString *){{.*}}= nil
// CHECK: (lldb) expression NSString
-// CHECK-NEXT: error:{{.*}} use of undeclared identifier 'NSString'
+// CHECK: error:{{.*}}use of undeclared identifier 'NSString'
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
index a249057282d890..8c168286903014 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
@@ -13,9 +13,7 @@
// CHECK: (lldb) expression d
// CHECK: (D) $1 = {}
// CHECK: (lldb) expression static_e_ref
-// CHECK: error: {{.*}}incomplete type 'E' where a complete type is required
-// CHECK: static_e_ref
-// CHECK: ^
+// CHECK: error:{{.*}}incomplete type 'E' where a complete type is required
// Complete base class.
struct A { int x; A(); };
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 14371da64f2f2f..afb1a1ff95c3a1 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -442,6 +442,7 @@ int Driver::MainLoop() {
m_debugger.SetInputFileHandle(stdin, false);
m_debugger.SetUseExternalEditor(m_option_data.m_use_external_editor);
+ m_debugger.SetShowInlineDiagnostics(true);
struct winsize window_size;
if ((isatty(STDIN_FILENO) != 0) &&
diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
index 4608b779f79a96..7e04d4b023e4c2 100644
--- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -72,18 +72,25 @@ TEST(DiagnosticManagerTest, HasFixits) {
EXPECT_TRUE(mgr.HasFixIts());
}
+static std::string toString(DiagnosticManager &mgr) {
+ // The error code doesn't really matter since we just convert the
+ // diagnostics to a string.
+ auto result = lldb::eExpressionCompleted;
+ return llvm::toString(mgr.GetAsError(result));
+}
+
TEST(DiagnosticManagerTest, GetStringNoDiags) {
DiagnosticManager mgr;
- EXPECT_EQ("", mgr.GetString());
+ EXPECT_EQ("", toString(mgr));
std::unique_ptr<Diagnostic> empty;
mgr.AddDiagnostic(std::move(empty));
- EXPECT_EQ("", mgr.GetString());
+ EXPECT_EQ("", toString(mgr));
}
TEST(DiagnosticManagerTest, GetStringBasic) {
DiagnosticManager mgr;
mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError));
- EXPECT_EQ("error: abc\n", mgr.GetString());
+ EXPECT_EQ("error: abc\n", toString(mgr));
}
TEST(DiagnosticManagerTest, GetStringMultiline) {
@@ -91,15 +98,15 @@ TEST(DiagnosticManagerTest, GetStringMultiline) {
// Multiline diagnostics should only get one severity label.
mgr.AddDiagnostic(std::make_unique<TextDiag>("b\nc", lldb::eSeverityError));
- EXPECT_EQ("error: b\nc\n", mgr.GetString());
+ EXPECT_EQ("error: b\nc\n", toString(mgr));
}
TEST(DiagnosticManagerTest, GetStringMultipleDiags) {
DiagnosticManager mgr;
mgr.AddDiagnostic(std::make_unique<TextDiag>("abc", lldb::eSeverityError));
- EXPECT_EQ("error: abc\n", mgr.GetString());
+ EXPECT_EQ("error: abc\n", toString(mgr));
mgr.AddDiagnostic(std::make_unique<TextDiag>("def", lldb::eSeverityError));
- EXPECT_EQ("error: abc\nerror: def\n", mgr.GetString());
+ EXPECT_EQ("error: abc\nerror: def\n", toString(mgr));
}
TEST(DiagnosticManagerTest, GetStringSeverityLabels) {
@@ -110,7 +117,7 @@ TEST(DiagnosticManagerTest, GetStringSeverityLabels) {
mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning));
// Remarks have no labels.
mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo));
- EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", mgr.GetString());
+ EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", toString(mgr));
}
TEST(DiagnosticManagerTest, GetStringPreserveOrder) {
@@ -120,7 +127,7 @@ TEST(DiagnosticManagerTest, GetStringPreserveOrder) {
mgr.AddDiagnostic(std::make_unique<TextDiag>("baz", lldb::eSeverityInfo));
mgr.AddDiagnostic(std::make_unique<TextDiag>("bar", lldb::eSeverityWarning));
mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError));
- EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", mgr.GetString());
+ EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", toString(mgr));
}
TEST(DiagnosticManagerTest, AppendMessageNoDiag) {
@@ -139,7 +146,7 @@ TEST(DiagnosticManagerTest, AppendMessageAttachToLastDiag) {
// This should append to 'bar' and not to 'foo'.
mgr.AppendMessageToDiagnostic("message text");
- EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", mgr.GetString());
+ EXPECT_EQ("error: foo\nerror: bar\nmessage text\n", toString(mgr));
}
TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) {
@@ -150,7 +157,7 @@ TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) {
// Pushing another diag after the message should work fine.
mgr.AddDiagnostic(std::make_unique<TextDiag>("foo", lldb::eSeverityError));
- EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", mgr.GetString());
+ EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", toString(mgr));
}
TEST(DiagnosticManagerTest, PutString) {
@@ -159,7 +166,7 @@ TEST(DiagnosticManagerTest, PutString) {
mgr.PutString(lldb::eSeverityError, "foo");
EXPECT_EQ(1U, mgr.Diagnostics().size());
EXPECT_EQ(eDiagnosticOriginLLDB, mgr.Diagnostics().front()->getKind());
- EXPECT_EQ("error: foo\n", mgr.GetString());
+ EXPECT_EQ("error: foo\n", toString(mgr));
}
TEST(DiagnosticManagerTest, PutStringMultiple) {
@@ -169,7 +176,7 @@ TEST(DiagnosticManagerTest, PutStringMultiple) {
mgr.PutString(lldb::eSeverityError, "foo");
mgr.PutString(lldb::eSeverityError, "bar");
EXPECT_EQ(2U, mgr.Diagnostics().size());
- EXPECT_EQ("error: foo\nerror: bar\n", mgr.GetString());
+ EXPECT_EQ("error: foo\nerror: bar\n", toString(mgr));
}
TEST(DiagnosticManagerTest, PutStringSeverities) {
@@ -180,7 +187,7 @@ TEST(DiagnosticManagerTest, PutStringSeverities) {
mgr.PutString(lldb::eSeverityError, "foo");
mgr.PutString(lldb::eSeverityWarning, "bar");
EXPECT_EQ(2U, mgr.Diagnostics().size());
- EXPECT_EQ("error: foo\nwarning: bar\n", mgr.GetString());
+ EXPECT_EQ("error: foo\nwarning: bar\n", toString(mgr));
}
TEST(DiagnosticManagerTest, FixedExpression) {
diff --git a/lldb/unittests/Interpreter/CMakeLists.txt b/lldb/unittests/Interpreter/CMakeLists.txt
index 54cea995084d3d..14a7d6c5610388 100644
--- a/lldb/unittests/Interpreter/CMakeLists.txt
+++ b/lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,5 +1,6 @@
add_lldb_unittest(InterpreterTests
TestCommandPaths.cpp
+ TestCommandObjectExpression.cpp
TestCompletion.cpp
TestOptionArgParser.cpp
TestOptions.cpp
diff --git a/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
new file mode 100644
index 00000000000000..b0cf22d3cf0903
--- /dev/null
+++ b/lldb/unittests/Interpreter/TestCommandObjectExpression.cpp
@@ -0,0 +1,34 @@
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+namespace lldb_private {
+std::string RenderDiagnosticDetails(Stream &stream,
+ std::optional<uint16_t> offset_in_command,
+ bool multiline,
+ llvm::ArrayRef<DiagnosticDetail> details);
+}
+
+using namespace lldb_private;
+using namespace lldb;
+using llvm::StringRef;
+namespace {
+class ErrorDisplayTest : public ::testing::Test {};
+} // namespace
+
+static std::string Render(std::vector<DiagnosticDetail> details) {
+ StreamString stream;
+ RenderDiagnosticDetails(stream, 0, true, details);
+ return stream.GetData();
+}
+
+TEST_F(ErrorDisplayTest, RenderStatus) {
+ DiagnosticDetail::SourceLocation inline_loc;
+ inline_loc.in_user_input = true;
+ {
+ std::string result =
+ Render({DiagnosticDetail{inline_loc, eSeverityError, "foo", ""}});
+ ASSERT_TRUE(StringRef(result).contains("error:"));
+ ASSERT_TRUE(StringRef(result).contains("foo"));
+ }
+}
More information about the lldb-commits
mailing list