[llvm] [Error] Add non-consuming toString (PR #95375)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 13 02:11:17 PDT 2024
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/95375
There are some places that want to convert an Error to string, but still retain the original Error object, for example to emit a non-fatal warning.
This currently isn't possible, because the entire Error infra is move-based. And what people end up doing in this case is to move the Error... twice.
This patch introduces a toStringWithoutConsuming() function to accommodate this use case. This also requires some infrastructure that allows visiting Errors without consuming them.
>From 155acb6faf18c43ecb515260f30346372bd1c082 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 13 Jun 2024 10:48:32 +0200
Subject: [PATCH] [Error] Add non-consuming toString
There are some places that want to convert an Error to string,
but still retain the original Error object, for example to emit
a non-fatal warning.
This currently isn't possible, because the entire Error infra
is move-based. So what people end up doing is just move the
Error... twice.
This patch introduces a toStringWithoutConsuming() function to
accomodate this use case. This also requires some infrastructure
that allows visiting Errors without consuming them.
---
llvm/include/llvm/Support/Error.h | 28 ++++++++++++++++++++
llvm/lib/Support/Error.cpp | 8 ++++++
llvm/tools/dsymutil/DwarfLinkerForBinary.cpp | 4 +--
llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp | 4 +--
llvm/unittests/Support/ErrorTest.cpp | 10 ++++++-
5 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index 1fa0d8cb709cc..5120f6ab57c03 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -166,6 +166,9 @@ class [[nodiscard]] Error {
// handleErrors needs to be able to set the Checked flag.
template <typename... HandlerTs>
friend Error handleErrors(Error E, HandlerTs &&... Handlers);
+ // visitErrors needs direct access to the payload.
+ template <typename HandlerT>
+ friend void visitErrors(const Error &E, HandlerT H);
// Expected<T> needs to be able to steal the payload when constructed from an
// error.
@@ -369,6 +372,10 @@ class ErrorList final : public ErrorInfo<ErrorList> {
// ErrorList.
template <typename... HandlerTs>
friend Error handleErrors(Error E, HandlerTs &&... Handlers);
+ // visitErrors needs to be able to iterate the payload list of an
+ // ErrorList.
+ template <typename HandlerT>
+ friend void visitErrors(const Error &E, HandlerT H);
// joinErrors is implemented in terms of join.
friend Error joinErrors(Error, Error);
@@ -977,6 +984,23 @@ inline void handleAllErrors(Error E) {
cantFail(std::move(E));
}
+/// Visit all the ErrorInfo(s) contained in E by passing them to the respective
+/// handler, without consuming the error.
+template <typename HandlerT> void visitErrors(const Error &E, HandlerT H) {
+ const ErrorInfoBase *Payload = E.getPtr();
+ if (!Payload)
+ return;
+
+ if (Payload->isA<ErrorList>()) {
+ const ErrorList &List = static_cast<const ErrorList &>(*Payload);
+ for (const auto &P : List.Payloads)
+ H(*P);
+ return;
+ }
+
+ return H(*Payload);
+}
+
/// Handle any errors (if present) in an Expected<T>, then try a recovery path.
///
/// If the incoming value is a success value it is returned unmodified. If it
@@ -1031,6 +1055,10 @@ void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner = {});
/// is used to separate error messages.
std::string toString(Error E);
+/// Like toString(), but does not consume the error. This can be used to print
+/// a warning while retaining the original error object.
+std::string toStringWithoutConsuming(const Error &E);
+
/// Consume a Error without doing anything. This method should be used
/// only where an error can be considered a reasonable and expected return
/// value.
diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
index 34ec31e3b833f..93481ca916d2a 100644
--- a/llvm/lib/Support/Error.cpp
+++ b/llvm/lib/Support/Error.cpp
@@ -82,6 +82,14 @@ std::string toString(Error E) {
return join(Errors.begin(), Errors.end(), "\n");
}
+std::string toStringWithoutConsuming(const Error &E) {
+ SmallVector<std::string, 2> Errors;
+ visitErrors(E, [&Errors](const ErrorInfoBase &EI) {
+ Errors.push_back(EI.message());
+ });
+ return join(Errors.begin(), Errors.end(), "\n");
+}
+
std::error_code ErrorList::convertToErrorCode() const {
return std::error_code(static_cast<int>(ErrorErrorCode::MultipleErrors),
getErrorErrorCat());
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index 83473704398df..f6a35708dc076 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -147,7 +147,7 @@ DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
if (!ObjectEntry) {
auto Err = ObjectEntry.takeError();
reportWarning(Twine(Obj.getObjectFilename()) + ": " +
- toString(std::move(Err)),
+ toStringWithoutConsuming(Err),
Obj.getObjectFilename());
return errorToErrorCode(std::move(Err));
}
@@ -156,7 +156,7 @@ DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
if (!Object) {
auto Err = Object.takeError();
reportWarning(Twine(Obj.getObjectFilename()) + ": " +
- toString(std::move(Err)),
+ toStringWithoutConsuming(Err),
Obj.getObjectFilename());
return errorToErrorCode(std::move(Err));
}
diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
index 1cecbfc463fec..b2362ecb75703 100644
--- a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
@@ -1494,13 +1494,13 @@ Error DumpOutputStyle::dumpModuleSymsForPdb() {
if (auto EC = Visitor.visitSymbolStreamFiltered(ModS.getSymbolArray(),
Filter)) {
P.formatLine("Error while processing symbol records. {0}",
- toString(std::move(EC)));
+ toStringWithoutConsuming(EC));
return EC;
}
} else if (auto EC = Visitor.visitSymbolStream(ModS.getSymbolArray(),
SS.Offset)) {
P.formatLine("Error while processing symbol records. {0}",
- toString(std::move(EC)));
+ toStringWithoutConsuming(EC));
return EC;
}
return Error::success();
diff --git a/llvm/unittests/Support/ErrorTest.cpp b/llvm/unittests/Support/ErrorTest.cpp
index 5d866a67c0eaa..aac9f4afdac01 100644
--- a/llvm/unittests/Support/ErrorTest.cpp
+++ b/llvm/unittests/Support/ErrorTest.cpp
@@ -738,17 +738,25 @@ TEST(Error, ErrorCodeConversions) {
// Test that error messages work.
TEST(Error, ErrorMessage) {
- EXPECT_EQ(toString(Error::success()), "");
+ Error E0 = Error::success();
+ EXPECT_EQ(toStringWithoutConsuming(E0), "");
+ EXPECT_EQ(toString(std::move(E0)), "");
Error E1 = make_error<CustomError>(0);
+ EXPECT_EQ(toStringWithoutConsuming(E1), "CustomError {0}");
EXPECT_EQ(toString(std::move(E1)), "CustomError {0}");
Error E2 = make_error<CustomError>(0);
+ visitErrors(E2, [](const ErrorInfoBase &EI) {
+ EXPECT_EQ(EI.message(), "CustomError {0}");
+ });
handleAllErrors(std::move(E2), [](const CustomError &CE) {
EXPECT_EQ(CE.message(), "CustomError {0}");
});
Error E3 = joinErrors(make_error<CustomError>(0), make_error<CustomError>(1));
+ EXPECT_EQ(toStringWithoutConsuming(E3), "CustomError {0}\n"
+ "CustomError {1}");
EXPECT_EQ(toString(std::move(E3)), "CustomError {0}\n"
"CustomError {1}");
}
More information about the llvm-commits
mailing list