Add support for a custom error type in `ErrorOr` (#5834)

This doesn't split apart the current error type into one that tracks
location and one that doesn't, although that might be easier to do once
we have this.

Instead, this is primarily intended to support custom error types that
lazily materialize the error message in case that can be avoided by
completely handling the error. For example, many file system operations
are *expected* to produce errors even in the hot path and we don't want
to render `ENOENT` (for example) to a pretty string and instead will
directly query the error to understand and handle it in code.

The type parameter ordering isn't the most obvious, but helpfully allows
us to default the error type in a useful way.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

common/error.h

@@ -5,9 +5,11 @@
 #ifndef CARBON_COMMON_ERROR_H_
 #define CARBON_COMMON_ERROR_H_
 
+#include <concepts>
 #include <functional>
 #include <string>
 #include <type_traits>
+#include <utility>
 #include <variant>
 
 #include "common/check.h"
@@ -71,11 +73,49 @@ class [[nodiscard]] Error : public Printable<Error> {
   std::string message_;
 };
 
-// Holds a value of type `T`, or an Error explaining why the value is
+// A common base class that custom error types should derive from.
+//
+// This combines the ability to be printed with the ability to convert the error
+// to a string and in turn to a non-customized `Error` type by rendering into a
+// string.
+//
+// The goal is that custom error types can be used for errors that are common
+// and/or would have cost to fully render the error message to a string. A
+// custom type can then be used to allow custom, light-weight handling of errors
+// when appropriate. But to avoid these custom types being excessively viral, we
+// ensure they can be converted to normal `Error` types when needed by rendering
+// fully to a string.
+template <typename ErrorT>
+class [[nodiscard]] ErrorBase : public Printable<ErrorT> {
+ public:
+  ErrorBase(const ErrorBase&) = delete;
+  auto operator=(const ErrorBase&) -> ErrorBase& = delete;
+
+  auto ToString() const -> std::string {
+    RawStringOstream os;
+    static_cast<const ErrorT*>(this)->Print(os);
+    return os.TakeStr();
+  }
+  auto ToError() const -> Error { return Error(this->ToString()); }
+
+ protected:
+  ErrorBase() = default;
+  ErrorBase(ErrorBase&&) noexcept = default;
+  auto operator=(ErrorBase&&) noexcept -> ErrorBase& = default;
+};
+
+// Holds a value of type `T`, or an `ErrorT` type explaining why the value is
 // unavailable.
 //
+// The `ErrorT` type defaults to `Error` but can be customized where desired
+// with a type that derives from `ErrorBase` above. See the documentation for
+// `ErrorBase` to understand the expected contract of custom error types.
+//
 // This is nodiscard to enforce error handling prior to destruction.
-template <typename T>
+template <typename T, typename ErrorT = Error>
+  requires(!std::is_reference_v<ErrorT> &&
+           (std::same_as<ErrorT, Error> ||
+            std::derived_from<ErrorT, ErrorBase<ErrorT>>))
 class [[nodiscard]] ErrorOr {
  public:
   using ValueT = std::remove_reference_t<T>;
@@ -83,7 +123,30 @@ class [[nodiscard]] ErrorOr {
   // Constructs with an error; the error must not be Error::Success().
   // Implicit for easy construction on returns.
   // NOLINTNEXTLINE(google-explicit-constructor)
-  ErrorOr(Error err) : val_(std::move(err)) {}
+  ErrorOr(ErrorT err) : val_(std::move(err)) {}
+
+  // Constructs from a custom error type derived from `ErrorBase` into an
+  // `ErrorOr` for `Error` to facilitate returning errors transparently.
+  template <typename OtherErrorT>
+    requires(std::same_as<ErrorT, Error> &&
+             std::derived_from<OtherErrorT, ErrorBase<OtherErrorT>>)
+  // Implicit for easy construction on returns.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ErrorOr(OtherErrorT other_err) : val_(other_err.ToError()) {}
+
+  // Constructs with any convertible error type, necessary for return statements
+  // that are already converting to the `ErrorOr` wrapper.
+  //
+  // This supports *explicitly* conversions, not just implicit, which is
+  // important to make common patterns of returning and adjusting the error
+  // type without each error type conversion needing to be implicit.
+  template <typename OtherErrorT>
+    requires(std::constructible_from<ErrorT, OtherErrorT> &&
+             std::derived_from<OtherErrorT, ErrorBase<OtherErrorT>>)
+  // Implicit for easy construction on returns.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ErrorOr(OtherErrorT other_err)
+      : val_(std::in_place_type<ErrorT>, std::move(other_err)) {}
 
   // Constructs with a reference.
   // Implicit for easy construction on returns.
@@ -104,13 +167,13 @@ class [[nodiscard]] ErrorOr {
 
   // Returns the contained error.
   // REQUIRES: `ok()` is false.
-  auto error() const& -> const Error& {
+  auto error() const& -> const ErrorT& {
     CARBON_CHECK(!ok());
-    return std::get<Error>(val_);
+    return std::get<ErrorT>(val_);
   }
-  auto error() && -> Error {
+  auto error() && -> ErrorT {
     CARBON_CHECK(!ok());
-    return std::get<Error>(std::move(val_));
+    return std::get<ErrorT>(std::move(val_));
   }
 
   // Returns the contained value.
@@ -140,7 +203,7 @@ class [[nodiscard]] ErrorOr {
                                      std::reference_wrapper<ValueT>, T>;
 
   // Either an error message or a value.
-  std::variant<Error, StoredT> val_;
+  std::variant<ErrorT, StoredT> val_;
 };
 
 // A helper class for accumulating error message and converting to

common/error_test.cpp

@@ -6,6 +6,8 @@
 
 #include <gtest/gtest.h>
 
+#include <concepts>
+
 #include "common/error_test_helpers.h"
 #include "common/raw_string_ostream.h"
 
@@ -29,94 +31,195 @@ auto IndirectError() -> Error { return Error("test"); }
 
 TEST(ErrorTest, IndirectError) { EXPECT_EQ(IndirectError().message(), "test"); }
 
-TEST(ErrorTest, ErrorOr) {
-  ErrorOr<int> err(Error("test"));
+TEST(ErrorTest, ErrorBuilderOperatorImplicitCast) {
+  ErrorOr<int> result = ErrorBuilder() << "msg";
+  EXPECT_THAT(result, IsError("msg"));
+}
 
-  EXPECT_THAT(err, IsError("test"));
+TEST(ErrorTest, StreamError) {
+  Error result = ErrorBuilder("TestFunc") << "msg";
+  RawStringOstream result_stream;
+  result_stream << result;
+  EXPECT_EQ(result_stream.TakeStr(), "TestFunc: msg");
 }
 
-TEST(ErrorTest, ErrorOrValue) { EXPECT_TRUE(ErrorOr<int>(0).ok()); }
+class CustomError : public ErrorBase<CustomError> {
+ public:
+  auto Print(llvm::raw_ostream& os) const -> void {
+    os << "Custom test error!";
+  }
+};
 
-auto IndirectErrorOrTest() -> ErrorOr<int> { return Error("test"); }
+template <typename ErrorT>
+class ErrorOrTest : public ::testing::Test {
+ public:
+  auto ErrorStr() -> std::string {
+    if constexpr (std::same_as<ErrorT, Error>) {
+      return "test error";
+    } else if constexpr (std::same_as<ErrorT, CustomError>) {
+      return CustomError().ToString();
+    } else {
+      static_assert(false, "Unsupported custom error type!");
+    }
+  }
+
+  auto MakeError() -> ErrorT {
+    if constexpr (std::same_as<ErrorT, Error>) {
+      return Error("test error");
+    } else if constexpr (std::same_as<ErrorT, CustomError>) {
+      return CustomError();
+    } else {
+      static_assert(false, "Unsupported custom error type!");
+    }
+  }
+};
 
-TEST(ErrorTest, IndirectErrorOr) { EXPECT_FALSE(IndirectErrorOrTest().ok()); }
+using ErrorOrTestParams = ::testing::Types<Error, CustomError>;
+TYPED_TEST_SUITE(ErrorOrTest, ErrorOrTestParams);
+
+TYPED_TEST(ErrorOrTest, ErrorOr) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  TestErrorOr err(this->MakeError());
+
+  EXPECT_THAT(err, IsError(this->ErrorStr()));
+}
+
+TYPED_TEST(ErrorOrTest, ErrorOrValue) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  EXPECT_TRUE(TestErrorOr(0).ok());
+}
+
+template <typename ErrorT, typename Fixture>
+auto IndirectErrorOrTest(Fixture& fixture) -> ErrorOr<int, ErrorT> {
+  return fixture.MakeError();
+}
+
+TYPED_TEST(ErrorOrTest, IndirectErrorOr) {
+  EXPECT_FALSE(IndirectErrorOrTest<TypeParam>(*this).ok());
+}
 
 struct Val {
   int val;
 };
 
-TEST(ErrorTest, ErrorOrArrowOp) {
-  ErrorOr<Val> err({1});
+TYPED_TEST(ErrorOrTest, ErrorOrArrowOp) {
+  using TestErrorOr = ErrorOr<Val, TypeParam>;
+  TestErrorOr err({1});
   EXPECT_EQ(err->val, 1);
 }
 
-TEST(ErrorTest, ErrorOrReference) {
+TYPED_TEST(ErrorOrTest, ErrorOrReference) {
+  using TestErrorOr = ErrorOr<Val&, TypeParam>;
   Val val = {1};
-  ErrorOr<Val&> maybe_val(val);
+  TestErrorOr maybe_val(val);
   EXPECT_EQ(maybe_val->val, 1);
 }
 
-auto IndirectErrorOrSuccessTest() -> ErrorOr<Success> { return Success(); }
+template <typename ErrorT>
+auto IndirectErrorOrSuccessTest() -> ErrorOr<Success, ErrorT> {
+  return Success();
+}
 
-TEST(ErrorTest, IndirectErrorOrSuccess) {
-  EXPECT_TRUE(IndirectErrorOrSuccessTest().ok());
+TYPED_TEST(ErrorOrTest, IndirectErrorOrSuccess) {
+  EXPECT_TRUE(IndirectErrorOrSuccessTest<TypeParam>().ok());
 }
 
-TEST(ErrorTest, ReturnIfErrorNoError) {
-  auto result = []() -> ErrorOr<Success> {
-    CARBON_RETURN_IF_ERROR(ErrorOr<Success>(Success()));
-    CARBON_RETURN_IF_ERROR(ErrorOr<Success>(Success()));
+TYPED_TEST(ErrorOrTest, ReturnIfErrorNoError) {
+  using TestErrorOr = ErrorOr<Success, TypeParam>;
+  auto result = []() -> TestErrorOr {
+    CARBON_RETURN_IF_ERROR(TestErrorOr(Success()));
+    CARBON_RETURN_IF_ERROR(TestErrorOr(Success()));
     return Success();
   }();
   EXPECT_TRUE(result.ok());
 }
 
-TEST(ErrorTest, ReturnIfErrorHasError) {
-  auto result = []() -> ErrorOr<Success> {
-    CARBON_RETURN_IF_ERROR(ErrorOr<Success>(Success()));
-    CARBON_RETURN_IF_ERROR(ErrorOr<Success>(Error("error")));
+TYPED_TEST(ErrorOrTest, ReturnIfErrorHasError) {
+  using TestErrorOr = ErrorOr<Success, TypeParam>;
+  auto result = [this]() -> TestErrorOr {
+    CARBON_RETURN_IF_ERROR(TestErrorOr(Success()));
+    CARBON_RETURN_IF_ERROR(TestErrorOr(this->MakeError()));
     return Success();
   }();
-  EXPECT_THAT(result, IsError("error"));
+  EXPECT_THAT(result, IsError(this->ErrorStr()));
 }
 
-TEST(ErrorTest, AssignOrReturnNoError) {
-  auto result = []() -> ErrorOr<int> {
-    CARBON_ASSIGN_OR_RETURN(int a, ErrorOr<int>(1));
-    CARBON_ASSIGN_OR_RETURN(const int b, ErrorOr<int>(2));
+TYPED_TEST(ErrorOrTest, AssignOrReturnNoError) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  auto result = []() -> TestErrorOr {
+    CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(1));
+    CARBON_ASSIGN_OR_RETURN(const int b, TestErrorOr(2));
     int c = 0;
-    CARBON_ASSIGN_OR_RETURN(c, ErrorOr<int>(3));
+    CARBON_ASSIGN_OR_RETURN(c, TestErrorOr(3));
     return a + b + c;
   }();
   EXPECT_THAT(result, IsSuccess(Eq(6)));
 }
 
-TEST(ErrorTest, AssignOrReturnHasDirectError) {
-  auto result = []() -> ErrorOr<int> {
-    CARBON_RETURN_IF_ERROR(ErrorOr<int>(Error("error")));
+TYPED_TEST(ErrorOrTest, AssignOrReturnHasDirectError) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  auto result = [this]() -> TestErrorOr {
+    CARBON_RETURN_IF_ERROR(TestErrorOr(this->MakeError()));
     return 0;
   }();
-  EXPECT_THAT(result, IsError("error"));
+  EXPECT_THAT(result, IsError(this->ErrorStr()));
 }
 
-TEST(ErrorTest, AssignOrReturnHasErrorInExpected) {
-  auto result = []() -> ErrorOr<int> {
-    CARBON_ASSIGN_OR_RETURN(int a, ErrorOr<int>(Error("error")));
+TYPED_TEST(ErrorOrTest, AssignOrReturnHasErrorInExpected) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  auto result = [this]() -> TestErrorOr {
+    CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(this->MakeError()));
     return a;
   }();
-  EXPECT_THAT(result, IsError("error"));
+  EXPECT_THAT(result, IsError(this->ErrorStr()));
 }
 
-TEST(ErrorTest, ErrorBuilderOperatorImplicitCast) {
-  ErrorOr<int> result = ErrorBuilder() << "msg";
-  EXPECT_THAT(result, IsError("msg"));
+class AnotherCustomError : public ErrorBase<AnotherCustomError> {
+ public:
+  auto Print(llvm::raw_ostream& os) const -> void {
+    os << "Another custom test error!";
+  }
+
+  explicit operator CustomError() { return CustomError(); }
+};
+
+TYPED_TEST(ErrorOrTest, AssignOrReturnNoErrorAcrossErrorTypes) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  auto result = []() -> ErrorOr<int> {
+    CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(1));
+    CARBON_ASSIGN_OR_RETURN(const int b, []() -> TestErrorOr {
+      CARBON_ASSIGN_OR_RETURN(int inner, (ErrorOr<int, AnotherCustomError>(2)));
+      return inner;
+    }());
+    int c = 0;
+    CARBON_ASSIGN_OR_RETURN(c, TestErrorOr(3));
+    return a + b + c;
+  }();
+  EXPECT_THAT(result, IsSuccess(Eq(6)));
 }
 
-TEST(ErrorTest, StreamError) {
-  Error result = ErrorBuilder("TestFunc") << "msg";
-  RawStringOstream result_stream;
-  result_stream << result;
-  EXPECT_EQ(result_stream.TakeStr(), "TestFunc: msg");
+TYPED_TEST(ErrorOrTest, AssignOrReturnErrorAcrossErrorTypes) {
+  using TestErrorOr = ErrorOr<int, TypeParam>;
+  auto result = []() -> ErrorOr<int> {
+    CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(1));
+    CARBON_ASSIGN_OR_RETURN(const int b, []() -> TestErrorOr {
+      CARBON_ASSIGN_OR_RETURN(
+          int inner, (ErrorOr<int, AnotherCustomError>(AnotherCustomError())));
+      return inner;
+    }());
+    int c = 0;
+    CARBON_ASSIGN_OR_RETURN(c, TestErrorOr(3));
+    return a + b + c;
+  }();
+
+  // When directly using the `Error` type, the explicit custom type above has
+  // its message preserved. When testing against `CustomError`, that one
+  // overrides the message.
+  if constexpr (std::same_as<TypeParam, Error>) {
+    EXPECT_THAT(result, IsError("Another custom test error!"));
+  } else {
+    EXPECT_THAT(result, IsError("Custom test error!"));
+  }
 }
 
 }  // namespace

common/error_test_helpers.h

@@ -21,14 +21,16 @@ class IsError {
   explicit IsError(::testing::Matcher<std::string> matcher)
       : matcher_(std::move(matcher)) {}
 
-  template <typename T>
-  auto MatchAndExplain(const ErrorOr<T>& result,
+  template <typename T, typename ErrorT>
+  auto MatchAndExplain(const ErrorOr<T, ErrorT>& result,
                        ::testing::MatchResultListener* listener) const -> bool {
     if (result.ok()) {
       *listener->stream() << "is a success";
       return false;
     } else {
-      return matcher_.MatchAndExplain(result.error().message(), listener);
+      RawStringOstream os;
+      os << result.error();
+      return matcher_.MatchAndExplain(os.TakeStr(), listener);
     }
   }
 
@@ -57,14 +59,13 @@ class IsSuccessMatcher {
   explicit IsSuccessMatcher(InnerMatcher matcher)
       : matcher_(std::move(matcher)) {}
 
-  template <typename T>
-  auto MatchAndExplain(const ErrorOr<T>& result,
+  template <typename T, typename ErrorT>
+  auto MatchAndExplain(const ErrorOr<T, ErrorT>& result,
                        ::testing::MatchResultListener* listener) const -> bool {
     if (result.ok()) {
       return ::testing::Matcher<T>(matcher_).MatchAndExplain(*result, listener);
     } else {
-      *listener->stream() << "is an error with `" << result.error().message()
-                          << "`";
+      *listener->stream() << "is an error with `" << result.error() << "`";
       return false;
     }
   }
@@ -94,14 +95,13 @@ auto IsSuccess(InnerMatcher matcher) -> IsSuccessMatcher<InnerMatcher> {
 namespace Carbon {
 
 // Supports printing `ErrorOr<T>` to `std::ostream` in tests.
-template <typename T>
-auto operator<<(std::ostream& out, const ErrorOr<T>& error_or)
+template <typename T, typename ErrorT>
+auto operator<<(std::ostream& out, const ErrorOr<T, ErrorT>& error_or)
     -> std::ostream& {
   if (error_or.ok()) {
     out << llvm::formatv("ErrorOr{{.value = `{0}`}}", *error_or);
   } else {
-    out << llvm::formatv("ErrorOr{{.error = \"{0}\"}}",
-                         error_or.error().message());
+    out << llvm::formatv("ErrorOr{{.error = \"{0}\"}}", error_or.error());
   }
   return out;
 }