[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

llvm-svn: 341064

Author: aganea

llvm/include/llvm/Support/Error.h

@@ -156,9 +156,10 @@ class ErrorInfoBase {
 /// 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... ArgTs> Error make_error(ArgTs &&... Args) {
 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> expectedToErrorOr(Expected<T> &&E) {
 /// 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 @@ class StringError : public ErrorInfo<StringError> {
 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 EC, char const *Fmt,
 
 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.

llvm/lib/Support/Error.cpp

@@ -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;

llvm/unittests/Support/ErrorTest.cpp

@@ -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