[llvm] r341064 - [Error] Add FileError helper; upgrade StringError behavior
Ilya Biryukov via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 30 06:59:48 PDT 2018
> Is there a way to preflight changes on the build system before commiting?
Not that I know of. We don't have presubmit checks for LLVM at the moment.
On Thu, Aug 30, 2018 at 3:20 PM Alexandre Ganea <alexandre.ganea at ubisoft.com>
wrote:
> Checking now.
>
>
>
> *De :* Ilya Biryukov <ibiryukov at google.com>
> *Envoyé :* 30 août 2018 09:19
> *À :* Alexandre Ganea <alexandre.ganea at ubisoft.com>
> *Cc :* llvm-commits <llvm-commits at lists.llvm.org>
> *Objet :* Re: [llvm] r341064 - [Error] Add FileError helper; upgrade
> StringError behavior
>
>
>
> This seems to break the build:
>
> ../include/llvm/Support/Error.h:1219:11: error: default arguments cannot
> be added to a function template that has already been declared
>
>
>
> Could you take a look please?
>
>
>
> On Thu, Aug 30, 2018 at 3:11 PM Alexandre Ganea via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> Author: aganea
> Date: Thu Aug 30 06:10:42 2018
> New Revision: 341064
>
> URL: http://llvm.org/viewvc/llvm-project?rev=341064&view=rev
> Log:
> [Error] Add FileError helper; upgrade StringError behavior
>
> FileError is meant to encapsulate both an Error and a file name/path. It
> should be used in cases where an Error occurs deep down the call chain, and
> we want to return it to the caller along with the file name.
>
> StringError was updated to display the error messages in different ways.
> These can be:
>
> 1. display the error_code message, and convert to the same error_code
> (ECError behavior)
> 2. display an arbitrary string, and convert to a provided error_code
> (current StringError behavior)
> 3. display both an error_code message and a string, in this order; and
> convert to the same error_code
>
> These behaviors can be triggered depending on the constructor. The goal is
> to use StringError as a base class, when a library needs to provide a
> explicit Error type.
>
> Differential Revision: https://reviews.llvm.org/D50807
>
> Modified:
> llvm/trunk/include/llvm/Support/Error.h
> llvm/trunk/lib/Support/Error.cpp
> llvm/trunk/unittests/Support/ErrorTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/Error.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Error.h?rev=341064&r1=341063&r2=341064&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/Error.h (original)
> +++ llvm/trunk/include/llvm/Support/Error.h Thu Aug 30 06:10:42 2018
> @@ -156,9 +156,10 @@ private:
> /// they're moved-assigned or constructed from Success values that have
> already
> /// been checked. This enforces checking through all levels of the call
> stack.
> class LLVM_NODISCARD Error {
> - // ErrorList needs to be able to yank ErrorInfoBase pointers out of this
> - // class to add to the error list.
> + // Both ErrorList and FileError need to be able to yank ErrorInfoBase
> + // pointers out of this class to add to the error list.
> friend class ErrorList;
> + friend class FileError;
>
> // handleErrors needs to be able to set the Checked flag.
> template <typename... HandlerTs>
> @@ -343,6 +344,8 @@ template <typename ErrT, typename... Arg
> template <typename ThisErrT, typename ParentErrT = ErrorInfoBase>
> class ErrorInfo : public ParentErrT {
> public:
> + using ParentErrT::ParentErrT; // inherit constructors
> +
> static const void *classID() { return &ThisErrT::ID; }
>
> const void *dynamicClassID() const override { return &ThisErrT::ID; }
> @@ -1110,10 +1113,33 @@ template <typename T> ErrorOr<T> expecte
> /// StringError is useful in cases where the client is not expected to be
> able
> /// to consume the specific error message programmatically (for example,
> if the
> /// error message is to be presented to the user).
> +///
> +/// StringError can also be used when additional information is to be
> printed
> +/// along with a error_code message. Depending on the constructor called,
> this
> +/// class can either display:
> +/// 1. the error_code message (ECError behavior)
> +/// 2. a string
> +/// 3. the error_code message and a string
> +///
> +/// These behaviors are useful when subtyping is required; for example,
> when a
> +/// specific library needs an explicit error type. In the example below,
> +/// PDBError is derived from StringError:
> +///
> +/// @code{.cpp}
> +/// Expected<int> foo() {
> +/// return
> llvm::make_error<PDBError>(pdb_error_code::dia_failed_loading,
> +/// "Additional information");
> +/// }
> +/// @endcode
> +///
> class StringError : public ErrorInfo<StringError> {
> public:
> static char ID;
>
> + // Prints EC + S and converts to EC
> + StringError(std::error_code EC, const Twine &S = Twine());
> +
> + // Prints S and converts to EC
> StringError(const Twine &S, std::error_code EC);
>
> void log(raw_ostream &OS) const override;
> @@ -1124,6 +1150,7 @@ public:
> private:
> std::string Msg;
> std::error_code EC;
> + const bool PrintMsgOnly = false;
> };
>
> /// Create formatted StringError object.
> @@ -1138,6 +1165,61 @@ Error createStringError(std::error_code
>
> Error createStringError(std::error_code EC, char const *Msg);
>
> +/// This class wraps a filename and another Error.
> +///
> +/// In some cases, an error needs to live along a 'source' name, in order
> to
> +/// show more detailed information to the user.
> +class FileError final : public ErrorInfo<FileError> {
> +
> + template <class Err>
> + friend Error createFileError(
> + std::string, Err,
> + typename std::enable_if<std::is_base_of<Error, Err>::value &&
> + !std::is_base_of<ErrorSuccess,
> Err>::value>::type
> + *);
> +
> +public:
> + void log(raw_ostream &OS) const override {
> + assert(Err && !FileName.empty() && "Trying to log after
> takeError().");
> + OS << "'" << FileName << "': ";
> + Err->log(OS);
> + }
> +
> + Error takeError() { return Error(std::move(Err)); }
> +
> + std::error_code convertToErrorCode() const override;
> +
> + // Used by ErrorInfo::classID.
> + static char ID;
> +
> +private:
> + FileError(std::string F, std::unique_ptr<ErrorInfoBase> E) {
> + assert(E && "Cannot create FileError from Error success value.");
> + assert(!F.empty() &&
> + "The file name provided to FileError must not be empty.");
> + FileName = F;
> + Err = std::move(E);
> + }
> +
> + static Error build(std::string F, Error E) {
> + return Error(std::unique_ptr<FileError>(new FileError(F,
> E.takePayload())));
> + }
> +
> + std::string FileName;
> + std::unique_ptr<ErrorInfoBase> Err;
> +};
> +
> +/// Concatenate a source file path and/or name with an Error. The
> resulting
> +/// Error is unchecked.
> +template <class Err>
> +inline Error createFileError(
> + std::string F, Err E,
> + typename std::enable_if<std::is_base_of<Error, Err>::value &&
> + !std::is_base_of<ErrorSuccess,
> Err>::value>::type
> + * = nullptr) {
> + return FileError::build(F, std::move(E));
> +}
> +
> /// Helper for check-and-exit error handling.
> ///
> /// For tool use only. NOT FOR USE IN LIBRARY CODE.
>
> Modified: llvm/trunk/lib/Support/Error.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Error.cpp?rev=341064&r1=341063&r2=341064&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/Error.cpp (original)
> +++ llvm/trunk/lib/Support/Error.cpp Thu Aug 30 06:10:42 2018
> @@ -19,6 +19,7 @@ namespace {
>
> enum class ErrorErrorCode : int {
> MultipleErrors = 1,
> + FileError,
> InconvertibleError
> };
>
> @@ -37,6 +38,8 @@ namespace {
> return "Inconvertible error value. An error has occurred that
> could "
> "not be converted to a known std::error_code. Please file
> a "
> "bug.";
> + case ErrorErrorCode::FileError:
> + return "A file error occurred.";
> }
> llvm_unreachable("Unhandled error code");
> }
> @@ -53,6 +56,7 @@ char ErrorInfoBase::ID = 0;
> char ErrorList::ID = 0;
> char ECError::ID = 0;
> char StringError::ID = 0;
> +char FileError::ID = 0;
>
> void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner) {
> if (!E)
> @@ -75,6 +79,11 @@ std::error_code inconvertibleErrorCode()
> *ErrorErrorCat);
> }
>
> +std::error_code FileError::convertToErrorCode() const {
> + return std::error_code(static_cast<int>(ErrorErrorCode::FileError),
> + *ErrorErrorCat);
> +}
> +
> Error errorCodeToError(std::error_code EC) {
> if (!EC)
> return Error::success();
> @@ -103,10 +112,21 @@ void Error::fatalUncheckedError() const
> }
> #endif
>
> -StringError::StringError(const Twine &S, std::error_code EC)
> +StringError::StringError(std::error_code EC, const Twine &S)
> : Msg(S.str()), EC(EC) {}
>
> -void StringError::log(raw_ostream &OS) const { OS << Msg; }
> +StringError::StringError(const Twine &S, std::error_code EC)
> + : Msg(S.str()), EC(EC), PrintMsgOnly(true) {}
> +
> +void StringError::log(raw_ostream &OS) const {
> + if (PrintMsgOnly) {
> + OS << Msg;
> + } else {
> + OS << EC.message();
> + if (!Msg.empty())
> + OS << (" " + Msg);
> + }
> +}
>
> std::error_code StringError::convertToErrorCode() const {
> return EC;
>
> Modified: llvm/trunk/unittests/Support/ErrorTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ErrorTest.cpp?rev=341064&r1=341063&r2=341064&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/ErrorTest.cpp (original)
> +++ llvm/trunk/unittests/Support/ErrorTest.cpp Thu Aug 30 06:10:42 2018
> @@ -13,6 +13,7 @@
> #include "llvm/ADT/Twine.h"
> #include "llvm/Support/Errc.h"
> #include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/ManagedStatic.h"
> #include "llvm/Testing/Support/Error.h"
> #include "gtest/gtest-spi.h"
> #include "gtest/gtest.h"
> @@ -104,7 +105,7 @@ TEST(Error, CheckedSuccess) {
> EXPECT_FALSE(E) << "Unexpected error while testing Error 'Success'";
> }
>
> -// Test that unchecked succes values cause an abort.
> +// Test that unchecked success values cause an abort.
> #if LLVM_ENABLE_ABI_BREAKING_CHECKS
> TEST(Error, UncheckedSuccess) {
> EXPECT_DEATH({ Error E = Error::success(); },
> @@ -864,4 +865,113 @@ TEST(Error, C_API) {
> EXPECT_TRUE(GotCE) << "Failed to round-trip ErrorList via C API";
> }
>
> +TEST(Error, FileErrorTest) {
> +#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
> + EXPECT_DEATH(
> + {
> + Error S = Error::success();
> + createFileError("file.bin", std::move(S));
> + },
> + "");
> +#endif
> + Error E1 = make_error<CustomError>(1);
> + Error FE1 = createFileError("file.bin", std::move(E1));
> + EXPECT_EQ(toString(std::move(FE1)).compare("'file.bin': CustomError
> {1}"), 0);
> +
> + Error E2 = make_error<CustomError>(2);
> + Error FE2 = createFileError("file.bin", std::move(E2));
> + handleAllErrors(std::move(FE2), [](const FileError &F) {
> + EXPECT_EQ(F.message().compare("'file.bin': CustomError {2}"), 0);
> + });
> +
> + Error E3 = make_error<CustomError>(3);
> + Error FE3 = createFileError("file.bin", std::move(E3));
> + auto E31 = handleErrors(std::move(FE3), [](std::unique_ptr<FileError>
> F) {
> + return F->takeError();
> + });
> + handleAllErrors(std::move(E31), [](const CustomError &C) {
> + EXPECT_EQ(C.message().compare("CustomError {3}"), 0);
> + });
> +
> + Error FE4 =
> + joinErrors(createFileError("file.bin", make_error<CustomError>(41)),
> + createFileError("file2.bin",
> make_error<CustomError>(42)));
> + EXPECT_EQ(toString(std::move(FE4))
> + .compare("'file.bin': CustomError {41}\n"
> + "'file2.bin': CustomError {42}"),
> + 0);
> +}
> +
> +enum class test_error_code {
> + unspecified = 1,
> + error_1,
> + error_2,
> +};
> +
> } // end anon namespace
> +
> +namespace std {
> + template <>
> + struct is_error_code_enum<test_error_code> : std::true_type {};
> +} // namespace std
> +
> +namespace {
> +
> +const std::error_category &TErrorCategory();
> +
> +inline std::error_code make_error_code(test_error_code E) {
> + return std::error_code(static_cast<int>(E), TErrorCategory());
> +}
> +
> +class TestDebugError : public ErrorInfo<TestDebugError, StringError> {
> +public:
> + using ErrorInfo<TestDebugError, StringError >::ErrorInfo; // inherit
> constructors
> + TestDebugError(const Twine &S) : ErrorInfo(S,
> test_error_code::unspecified) {}
> + static char ID;
> +};
> +
> +class TestErrorCategory : public std::error_category {
> +public:
> + const char *name() const noexcept override { return "error"; }
> + std::string message(int Condition) const override {
> + switch (static_cast<test_error_code>(Condition)) {
> + case test_error_code::unspecified:
> + return "An unknown error has occurred.";
> + case test_error_code::error_1:
> + return "Error 1.";
> + case test_error_code::error_2:
> + return "Error 2.";
> + }
> + llvm_unreachable("Unrecognized test_error_code");
> + }
> +};
> +
> +static llvm::ManagedStatic<TestErrorCategory> TestErrCategory;
> +const std::error_category &TErrorCategory() { return *TestErrCategory; }
> +
> +char TestDebugError::ID;
> +
> +TEST(Error, SubtypeStringErrorTest) {
> + auto E1 = make_error<TestDebugError>(test_error_code::error_1);
> + EXPECT_EQ(toString(std::move(E1)).compare("Error 1."), 0);
> +
> + auto E2 = make_error<TestDebugError>(test_error_code::error_1,
> + "Detailed information");
> + EXPECT_EQ(toString(std::move(E2)).compare("Error 1. Detailed
> information"),
> + 0);
> +
> + auto E3 = make_error<TestDebugError>(test_error_code::error_2);
> + handleAllErrors(std::move(E3), [](const TestDebugError &F) {
> + EXPECT_EQ(F.message().compare("Error 2."), 0);
> + });
> +
> + auto E4 =
> joinErrors(make_error<TestDebugError>(test_error_code::error_1,
> + "Detailed information"),
> +
> make_error<TestDebugError>(test_error_code::error_2));
> + EXPECT_EQ(toString(std::move(E4))
> + .compare("Error 1. Detailed information\n"
> + "Error 2."),
> + 0);
> +}
> +
> +} // namespace
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
>
> --
>
> Regards,
>
> Ilya Biryukov
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180830/12f2e0f8/attachment.html>
More information about the llvm-commits
mailing list