[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 20 10:53:39 PDT 2024
https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/106442
>From 510511a7a9e293c268c987e670646038bd96ff45 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Fri, 20 Sep 2024 10:53:09 -0700
Subject: [PATCH] [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 | 91 +++++++++++++-----
lldb/include/lldb/Utility/Status.h | 7 +-
lldb/source/Breakpoint/BreakpointLocation.cpp | 10 +-
lldb/source/Expression/DiagnosticManager.cpp | 94 +++++++++++++++----
lldb/source/Expression/ExpressionParser.cpp | 5 +-
lldb/source/Expression/UserExpression.cpp | 32 ++++---
lldb/source/Expression/UtilityFunction.cpp | 16 ++--
.../ExpressionParser/Clang/ClangDiagnostic.h | 5 +-
.../Clang/ClangExpressionParser.cpp | 69 ++++++++++----
.../Clang/ClangUserExpression.h | 2 +
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 10 +-
.../Platform/Windows/PlatformWindows.cpp | 10 +-
lldb/source/Target/Target.cpp | 2 +-
lldb/source/Utility/Status.cpp | 9 ++
.../TestModulesCompileError.py | 2 +-
.../Expression/DiagnosticManagerTest.cpp | 22 +++--
lldb/unittests/Utility/StatusTest.cpp | 2 +-
17 files changed, 269 insertions(+), 119 deletions(-)
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..1c0763eea92f05 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,52 @@
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 DetailedExpressionError
+ : public llvm::ErrorInfo<DetailedExpressionError, CloneableError> {
+ DiagnosticDetail m_detail;
+
+public:
+ using llvm::ErrorInfo<DetailedExpressionError, CloneableError>::ErrorInfo;
+ DetailedExpressionError(DiagnosticDetail detail) : m_detail(detail) {}
+ std::string message() const override;
+ DiagnosticDetail GetDetail() const { return m_detail; }
+ std::error_code convertToErrorCode() const override;
+ void log(llvm::raw_ostream &OS) const override;
+ std::unique_ptr<CloneableError> Clone() const override;
+
+ static char ID;
+};
+
enum DiagnosticOrigin {
eDiagnosticOriginUnknown = 0,
eDiagnosticOriginLLDB,
@@ -49,37 +98,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; }
+ llvm::Error GetAsError() const;
- 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 +142,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 +167,14 @@ 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) const;
+
+ /// Copies the diagnostics into an llvm::Error, the first diagnostic
+ /// being an llvm::StringError.
+ llvm::Error GetAsError(llvm::Twine msg) 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 795c830b965173..9343eb3097a471 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -115,8 +115,11 @@ class Status {
const Status &operator=(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()); }
@@ -136,7 +139,7 @@ class Status {
/// Clear the object state.
///
- /// Reverts the state of this object to contain a generic success value and
+ /// Reverts the state of this object to contain a generic successr value and
/// frees any cached error string value.
void Clear();
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 8d7364052a006a..fd9248eb0677c5 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -264,9 +264,9 @@ 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("Couldn't parse conditional expression:"));
+
m_user_expression_sp.reset();
return true;
}
@@ -324,8 +324,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("Couldn't execute expression:"));
}
return ret;
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index a8330138f3d53b..99533aefb685b1 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -14,22 +14,7 @@
#include "lldb/Utility/StreamString.h"
using namespace lldb_private;
-
-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();
- }
-
- log->PutCString(str.c_str());
-}
+char DetailedExpressionError::ID;
static const char *StringForSeverity(lldb::Severity severity) {
switch (severity) {
@@ -44,9 +29,28 @@ static const char *StringForSeverity(lldb::Severity severity) {
llvm_unreachable("switch needs another case for lldb::Severity enum");
}
+std::string DetailedExpressionError::message() const {
+ std::string str;
+ llvm::raw_string_ostream(str)
+ << StringForSeverity(m_detail.severity) << m_detail.rendered;
+ return str;
+}
+
+std::error_code DetailedExpressionError::convertToErrorCode() const {
+ return llvm::inconvertibleErrorCode();
+}
+
+void DetailedExpressionError::log(llvm::raw_ostream &OS) const {
+ OS << message();
+}
+
+std::unique_ptr<CloneableError> DetailedExpressionError::Clone() const {
+ return std::make_unique<DetailedExpressionError>(m_detail);
+}
+
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 +65,50 @@ std::string DiagnosticManager::GetString(char separator) {
stream << message.drop_front(severity_pos + severity.size());
stream << separator;
}
+ return str;
+}
- return ret;
+void DiagnosticManager::Dump(Log *log) {
+ if (!log)
+ return;
+
+ std::string str = GetString();
+
+ // We want to remove the last '\n' because log->PutCString will add
+ // one for us.
+
+ if (str.size() && str.back() == '\n')
+ str.pop_back();
+
+ log->PutString(str);
+}
+
+llvm::Error Diagnostic::GetAsError() const {
+ return llvm::make_error<DetailedExpressionError>(m_detail);
+}
+
+llvm::Error
+DiagnosticManager::GetAsError(lldb::ExpressionResults result) const {
+ llvm::Error diags = Status::FromExpressionError(result, "").takeError();
+ for (const auto &diagnostic : m_diagnostics)
+ diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+ return diags;
+}
+
+llvm::Error DiagnosticManager::GetAsError(llvm::Twine msg) const {
+ llvm::Error diags = llvm::createStringError(msg);
+ for (const auto &diagnostic : m_diagnostics)
+ diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+ return diags;
+}
+
+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 +131,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..26bc19874c53cc 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("couldn't run static initializer:"));
return err;
}
}
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 872f6304f91baa..55ba857db88db6 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::FromExpressionError(
+ execution_results,
+ "expression failed to parse (no further compiler diagnostics)");
+ else
+ error =
+ Status::FromError(diagnostic_manager.GetAsError(execution_results));
}
}
@@ -351,7 +353,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
LLDB_LOG(log, "== [UserExpression::Evaluate] Expression may not run, but "
"is not constant ==");
- if (!diagnostic_manager.Diagnostics().size())
+ if (diagnostic_manager.Diagnostics().empty())
error = Status::FromExpressionError(
lldb::eExpressionSetupError,
"expression needed to run but couldn't");
@@ -380,12 +382,12 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
LLDB_LOG(log, "== [UserExpression::Evaluate] Execution completed "
"abnormally ==");
- if (!diagnostic_manager.Diagnostics().size())
+ if (diagnostic_manager.Diagnostics().empty())
error = Status::FromExpressionError(
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();
diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp
index 55ebfb8ef342e8..e35c0774815298 100644
--- a/lldb/source/Expression/UtilityFunction.cpp
+++ b/lldb/source/Expression/UtilityFunction.cpp
@@ -83,19 +83,18 @@ 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(
+ "Error compiling " + m_function_name + " caller function:"));
+
m_caller_up.reset();
return nullptr;
}
@@ -104,9 +103,8 @@ 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(
+ "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..92bd554767dae2 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("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..5d873bf73da260 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -863,9 +863,8 @@ 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(
+ "dlopen error: could not write function arguments:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
@@ -906,9 +905,8 @@ 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(
+ "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..fa39d93a91297c 100644
--- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -341,9 +341,8 @@ 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(
+ "LoadLibrary error: unable to write function parameters:"));
return LLDB_INVALID_IMAGE_TOKEN;
}
@@ -384,9 +383,8 @@ 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(
+ "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 f1659aae0800db..57061ecd5bd445 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2605,7 +2605,7 @@ 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("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 4af3af5fba0185..b23c3403e92552 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -170,6 +170,15 @@ const char *Status::AsCString(const char *default_error_str) const {
break;
}
}
+
+ // FIXME: Workaround for ErrorList[ExpressionError, ...].
+ if (m_error.isA<llvm::ErrorList>()) {
+ while (!m_string.empty() && m_string[0] == '\n')
+ m_string = std::string(m_string.data() + 1, m_string.size() - 1);
+ if (!m_string.empty() && m_string[m_string.size() - 1] != '\n')
+ m_string += '\n';
+ }
+
if (m_string.empty()) {
if (default_error_str)
m_string.assign(default_error_str);
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()));
+}
diff --git a/lldb/unittests/Utility/StatusTest.cpp b/lldb/unittests/Utility/StatusTest.cpp
index be4f2beebcdb52..909e745c2082e0 100644
--- a/lldb/unittests/Utility/StatusTest.cpp
+++ b/lldb/unittests/Utility/StatusTest.cpp
@@ -55,7 +55,7 @@ TEST(StatusTest, ErrorCodeConstructor) {
llvm::Error list = llvm::joinErrors(llvm::createStringError("foo"),
llvm::createStringError("bar"));
Status foobar = Status::FromError(std::move(list));
- EXPECT_EQ(std::string("foo\nbar"), std::string(foobar.AsCString()));
+ EXPECT_EQ(std::string("foo\nbar\n"), std::string(foobar.AsCString()));
}
TEST(StatusTest, ErrorConversion) {
More information about the lldb-commits
mailing list