diff --git a/common/error.h b/common/error.h index 72486e5821e1..641f77fa771f 100644 --- a/common/error.h +++ b/common/error.h @@ -5,9 +5,11 @@ #ifndef CARBON_COMMON_ERROR_H_ #define CARBON_COMMON_ERROR_H_ +#include #include #include #include +#include #include #include "common/check.h" @@ -71,11 +73,49 @@ class [[nodiscard]] Error : public Printable { 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 +class [[nodiscard]] ErrorBase : public Printable { + public: + ErrorBase(const ErrorBase&) = delete; + auto operator=(const ErrorBase&) -> ErrorBase& = delete; + + auto ToString() const -> std::string { + RawStringOstream os; + static_cast(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 +template + requires(!std::is_reference_v && + (std::same_as || + std::derived_from>)) class [[nodiscard]] ErrorOr { public: using ValueT = std::remove_reference_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 + requires(std::same_as && + std::derived_from>) + // 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 + requires(std::constructible_from && + std::derived_from>) + // Implicit for easy construction on returns. + // NOLINTNEXTLINE(google-explicit-constructor) + ErrorOr(OtherErrorT other_err) + : val_(std::in_place_type, 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(val_); + return std::get(val_); } - auto error() && -> Error { + auto error() && -> ErrorT { CARBON_CHECK(!ok()); - return std::get(std::move(val_)); + return std::get(std::move(val_)); } // Returns the contained value. @@ -140,7 +203,7 @@ class [[nodiscard]] ErrorOr { std::reference_wrapper, T>; // Either an error message or a value. - std::variant val_; + std::variant val_; }; // A helper class for accumulating error message and converting to diff --git a/common/error_test.cpp b/common/error_test.cpp index 1e5dac824fae..67216f033ee0 100644 --- a/common/error_test.cpp +++ b/common/error_test.cpp @@ -6,6 +6,8 @@ #include +#include + #include "common/error_test_helpers.h" #include "common/raw_string_ostream.h" @@ -29,84 +31,6 @@ auto IndirectError() -> Error { return Error("test"); } TEST(ErrorTest, IndirectError) { EXPECT_EQ(IndirectError().message(), "test"); } -TEST(ErrorTest, ErrorOr) { - ErrorOr err(Error("test")); - - EXPECT_THAT(err, IsError("test")); -} - -TEST(ErrorTest, ErrorOrValue) { EXPECT_TRUE(ErrorOr(0).ok()); } - -auto IndirectErrorOrTest() -> ErrorOr { return Error("test"); } - -TEST(ErrorTest, IndirectErrorOr) { EXPECT_FALSE(IndirectErrorOrTest().ok()); } - -struct Val { - int val; -}; - -TEST(ErrorTest, ErrorOrArrowOp) { - ErrorOr err({1}); - EXPECT_EQ(err->val, 1); -} - -TEST(ErrorTest, ErrorOrReference) { - Val val = {1}; - ErrorOr maybe_val(val); - EXPECT_EQ(maybe_val->val, 1); -} - -auto IndirectErrorOrSuccessTest() -> ErrorOr { return Success(); } - -TEST(ErrorTest, IndirectErrorOrSuccess) { - EXPECT_TRUE(IndirectErrorOrSuccessTest().ok()); -} - -TEST(ErrorTest, ReturnIfErrorNoError) { - auto result = []() -> ErrorOr { - CARBON_RETURN_IF_ERROR(ErrorOr(Success())); - CARBON_RETURN_IF_ERROR(ErrorOr(Success())); - return Success(); - }(); - EXPECT_TRUE(result.ok()); -} - -TEST(ErrorTest, ReturnIfErrorHasError) { - auto result = []() -> ErrorOr { - CARBON_RETURN_IF_ERROR(ErrorOr(Success())); - CARBON_RETURN_IF_ERROR(ErrorOr(Error("error"))); - return Success(); - }(); - EXPECT_THAT(result, IsError("error")); -} - -TEST(ErrorTest, AssignOrReturnNoError) { - auto result = []() -> ErrorOr { - CARBON_ASSIGN_OR_RETURN(int a, ErrorOr(1)); - CARBON_ASSIGN_OR_RETURN(const int b, ErrorOr(2)); - int c = 0; - CARBON_ASSIGN_OR_RETURN(c, ErrorOr(3)); - return a + b + c; - }(); - EXPECT_THAT(result, IsSuccess(Eq(6))); -} - -TEST(ErrorTest, AssignOrReturnHasDirectError) { - auto result = []() -> ErrorOr { - CARBON_RETURN_IF_ERROR(ErrorOr(Error("error"))); - return 0; - }(); - EXPECT_THAT(result, IsError("error")); -} - -TEST(ErrorTest, AssignOrReturnHasErrorInExpected) { - auto result = []() -> ErrorOr { - CARBON_ASSIGN_OR_RETURN(int a, ErrorOr(Error("error"))); - return a; - }(); - EXPECT_THAT(result, IsError("error")); -} - TEST(ErrorTest, ErrorBuilderOperatorImplicitCast) { ErrorOr result = ErrorBuilder() << "msg"; EXPECT_THAT(result, IsError("msg")); @@ -119,5 +43,184 @@ TEST(ErrorTest, StreamError) { EXPECT_EQ(result_stream.TakeStr(), "TestFunc: msg"); } +class CustomError : public ErrorBase { + public: + auto Print(llvm::raw_ostream& os) const -> void { + os << "Custom test error!"; + } +}; + +template +class ErrorOrTest : public ::testing::Test { + public: + auto ErrorStr() -> std::string { + if constexpr (std::same_as) { + return "test error"; + } else if constexpr (std::same_as) { + return CustomError().ToString(); + } else { + static_assert(false, "Unsupported custom error type!"); + } + } + + auto MakeError() -> ErrorT { + if constexpr (std::same_as) { + return Error("test error"); + } else if constexpr (std::same_as) { + return CustomError(); + } else { + static_assert(false, "Unsupported custom error type!"); + } + } +}; + +using ErrorOrTestParams = ::testing::Types; +TYPED_TEST_SUITE(ErrorOrTest, ErrorOrTestParams); + +TYPED_TEST(ErrorOrTest, ErrorOr) { + using TestErrorOr = ErrorOr; + TestErrorOr err(this->MakeError()); + + EXPECT_THAT(err, IsError(this->ErrorStr())); +} + +TYPED_TEST(ErrorOrTest, ErrorOrValue) { + using TestErrorOr = ErrorOr; + EXPECT_TRUE(TestErrorOr(0).ok()); +} + +template +auto IndirectErrorOrTest(Fixture& fixture) -> ErrorOr { + return fixture.MakeError(); +} + +TYPED_TEST(ErrorOrTest, IndirectErrorOr) { + EXPECT_FALSE(IndirectErrorOrTest(*this).ok()); +} + +struct Val { + int val; +}; + +TYPED_TEST(ErrorOrTest, ErrorOrArrowOp) { + using TestErrorOr = ErrorOr; + TestErrorOr err({1}); + EXPECT_EQ(err->val, 1); +} + +TYPED_TEST(ErrorOrTest, ErrorOrReference) { + using TestErrorOr = ErrorOr; + Val val = {1}; + TestErrorOr maybe_val(val); + EXPECT_EQ(maybe_val->val, 1); +} + +template +auto IndirectErrorOrSuccessTest() -> ErrorOr { + return Success(); +} + +TYPED_TEST(ErrorOrTest, IndirectErrorOrSuccess) { + EXPECT_TRUE(IndirectErrorOrSuccessTest().ok()); +} + +TYPED_TEST(ErrorOrTest, ReturnIfErrorNoError) { + using TestErrorOr = ErrorOr; + auto result = []() -> TestErrorOr { + CARBON_RETURN_IF_ERROR(TestErrorOr(Success())); + CARBON_RETURN_IF_ERROR(TestErrorOr(Success())); + return Success(); + }(); + EXPECT_TRUE(result.ok()); +} + +TYPED_TEST(ErrorOrTest, ReturnIfErrorHasError) { + using TestErrorOr = ErrorOr; + auto result = [this]() -> TestErrorOr { + CARBON_RETURN_IF_ERROR(TestErrorOr(Success())); + CARBON_RETURN_IF_ERROR(TestErrorOr(this->MakeError())); + return Success(); + }(); + EXPECT_THAT(result, IsError(this->ErrorStr())); +} + +TYPED_TEST(ErrorOrTest, AssignOrReturnNoError) { + using TestErrorOr = ErrorOr; + 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, TestErrorOr(3)); + return a + b + c; + }(); + EXPECT_THAT(result, IsSuccess(Eq(6))); +} + +TYPED_TEST(ErrorOrTest, AssignOrReturnHasDirectError) { + using TestErrorOr = ErrorOr; + auto result = [this]() -> TestErrorOr { + CARBON_RETURN_IF_ERROR(TestErrorOr(this->MakeError())); + return 0; + }(); + EXPECT_THAT(result, IsError(this->ErrorStr())); +} + +TYPED_TEST(ErrorOrTest, AssignOrReturnHasErrorInExpected) { + using TestErrorOr = ErrorOr; + auto result = [this]() -> TestErrorOr { + CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(this->MakeError())); + return a; + }(); + EXPECT_THAT(result, IsError(this->ErrorStr())); +} + +class AnotherCustomError : public ErrorBase { + 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; + auto result = []() -> ErrorOr { + CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(1)); + CARBON_ASSIGN_OR_RETURN(const int b, []() -> TestErrorOr { + CARBON_ASSIGN_OR_RETURN(int inner, (ErrorOr(2))); + return inner; + }()); + int c = 0; + CARBON_ASSIGN_OR_RETURN(c, TestErrorOr(3)); + return a + b + c; + }(); + EXPECT_THAT(result, IsSuccess(Eq(6))); +} + +TYPED_TEST(ErrorOrTest, AssignOrReturnErrorAcrossErrorTypes) { + using TestErrorOr = ErrorOr; + auto result = []() -> ErrorOr { + CARBON_ASSIGN_OR_RETURN(int a, TestErrorOr(1)); + CARBON_ASSIGN_OR_RETURN(const int b, []() -> TestErrorOr { + CARBON_ASSIGN_OR_RETURN( + int inner, (ErrorOr(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) { + EXPECT_THAT(result, IsError("Another custom test error!")); + } else { + EXPECT_THAT(result, IsError("Custom test error!")); + } +} + } // namespace } // namespace Carbon diff --git a/common/error_test_helpers.h b/common/error_test_helpers.h index 3e87cfa87f6c..ecc368241566 100644 --- a/common/error_test_helpers.h +++ b/common/error_test_helpers.h @@ -21,14 +21,16 @@ class IsError { explicit IsError(::testing::Matcher matcher) : matcher_(std::move(matcher)) {} - template - auto MatchAndExplain(const ErrorOr& result, + template + auto MatchAndExplain(const ErrorOr& 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 - auto MatchAndExplain(const ErrorOr& result, + template + auto MatchAndExplain(const ErrorOr& result, ::testing::MatchResultListener* listener) const -> bool { if (result.ok()) { return ::testing::Matcher(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 { namespace Carbon { // Supports printing `ErrorOr` to `std::ostream` in tests. -template -auto operator<<(std::ostream& out, const ErrorOr& error_or) +template +auto operator<<(std::ostream& out, const ErrorOr& 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; }