diff --git a/README.md b/README.md index 67ad343..3547086 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ Implemented C++ Standard Proposals: - [x] [P0323R12](https://wg21.link/p0323r12) `` - [x] [P2505R5](https://wg21.link/p2505r5) Monadic Functions For expected - [x] [P2549R1](https://wg21.link/p2549r1) `std::unexpected` should have `error()` as member accessor +- [x] [P3379R0](https://wg21.link/p3379r0) Constrain `std::expected` equality operators Implemented LWG Issues: @@ -25,6 +26,7 @@ Implemented LWG Issues: - [x] [LWG-4031](https://wg21.link/lwg4031) `bad_expected_access` member functions should be noexcept - [x] [LWG-4222](https://wg21.link/lwg4222) `expected` constructor from a single value missing a constraint - [x] [LWG-4025](https://wg21.link/lwg4025) Move assignment operator of `std::expected` should not be conditionally deleted +- [x] [LWG-4366](https://wg21.link/lwg4366) Heterogeneous comparison of `expected` may be ill-formed Enhancements: diff --git a/include/zeus/expected.hpp b/include/zeus/expected.hpp index bc36dcc..39d24b5 100644 --- a/include/zeus/expected.hpp +++ b/include/zeus/expected.hpp @@ -130,6 +130,42 @@ inline constexpr bool is_move_assignable_or_void_v = is_void_or_v inline constexpr bool is_nothrow_convertible_v = noexcept(static_cast(std::declval())); +template +struct is_equality_result_convertible_to_bool : std::false_type +{ +}; + +template +struct is_equality_result_convertible_to_bool() == std::declval())>> + : std::is_convertible() == std::declval()), bool> +{ +}; + +template +inline constexpr bool is_equality_result_convertible_to_bool_v = is_equality_result_convertible_to_bool::value; + +constexpr bool implicitly_convert_to_bool(bool value) noexcept +{ + return value; +} + +template +struct is_nothrow_equality_result_convertible_to_bool : std::false_type +{ +}; + +template +struct is_nothrow_equality_result_convertible_to_bool< + Lhs, + Rhs, + std::void_t() == std::declval()))> +> : std::bool_constant() == std::declval()))> +{ +}; + +template +inline constexpr bool is_nothrow_equality_result_convertible_to_bool_v = is_nothrow_equality_result_convertible_to_bool::value; + } // namespace expected_detail template @@ -2037,9 +2073,16 @@ class expected } template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator==( - const expected &x, const expected &y - ) noexcept(noexcept(*x == *y) && noexcept(x.error() == y.error())) + [[nodiscard]] friend constexpr std::enable_if_t< + !std::is_void_v && + expected_detail::is_equality_result_convertible_to_bool_v && + expected_detail::is_equality_result_convertible_to_bool_v, + bool + > + operator==(const expected &x, const expected &y) noexcept( + expected_detail::is_nothrow_equality_result_convertible_to_bool_v && + expected_detail::is_nothrow_equality_result_convertible_to_bool_v + ) { if (x.has_value() != y.has_value()) { @@ -2056,20 +2099,26 @@ class expected } #if ZEUS_EXPECTED_CPLUSPLUS < 202'002L template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator!=( - const expected &x, const expected &y - ) noexcept(noexcept(x == y)) + [[nodiscard]] friend constexpr std::enable_if_t< + !std::is_void_v && + expected_detail::is_equality_result_convertible_to_bool_v && + expected_detail::is_equality_result_convertible_to_bool_v, + bool + > operator!=(const expected &x, const expected &y) noexcept(noexcept(x == y)) { return !(x == y); } #endif template - [[nodiscard]] friend constexpr bool operator==(const expected &x, const T2 &v) noexcept(noexcept(*x == v)) + [[nodiscard]] friend constexpr std::enable_if_t< + !expected_detail::is_specialization_v && expected_detail::is_equality_result_convertible_to_bool_v, + bool + > operator==(const expected &x, const T2 &v) noexcept(expected_detail::is_nothrow_equality_result_convertible_to_bool_v) { if (x.has_value()) { - return static_cast(*x == v); + return *x == v; } else { @@ -2078,28 +2127,19 @@ class expected } #if ZEUS_EXPECTED_CPLUSPLUS < 202'002L template - [[nodiscard]] friend constexpr bool operator!=(const expected &x, const T2 &v) noexcept(noexcept(x == v)) + [[nodiscard]] friend constexpr std::enable_if_t< + !expected_detail::is_specialization_v && expected_detail::is_equality_result_convertible_to_bool_v, + bool + > operator!=(const expected &x, const T2 &v) noexcept(noexcept(x == v)) { return !(x == v); } - template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator==( - const T2 &v, const expected &x - ) noexcept(noexcept(x == v)) - { - return x == v; - } - template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator!=( - const T2 &v, const expected &x - ) noexcept(noexcept(x == v)) - { - return x != v; - } #endif template - [[nodiscard]] friend constexpr bool operator==(const expected &x, const unexpected &e) noexcept(noexcept(x.error() == e.error())) + [[nodiscard]] friend constexpr std::enable_if_t, bool> operator==( + const expected &x, const unexpected &e + ) noexcept(expected_detail::is_nothrow_equality_result_convertible_to_bool_v) { if (x.has_value()) { @@ -2107,25 +2147,17 @@ class expected } else { - return static_cast(x.error() == e.error()); + return x.error() == e.error(); } } #if ZEUS_EXPECTED_CPLUSPLUS < 202'002L template - [[nodiscard]] friend constexpr bool operator!=(const expected &x, const unexpected &e) noexcept(noexcept(x == e)) + [[nodiscard]] friend constexpr std::enable_if_t, bool> operator!=( + const expected &x, const unexpected &e + ) noexcept(noexcept(x == e)) { return !(x == e); } - template - [[nodiscard]] friend constexpr bool operator==(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) - { - return x == e; - } - template - [[nodiscard]] friend constexpr bool operator!=(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) - { - return x != e; - } #endif }; @@ -2743,31 +2775,40 @@ class expected } template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator==( - const expected &x, const expected &y - ) noexcept(noexcept(x.error() == y.error())) + [[nodiscard]] friend constexpr std:: + enable_if_t && expected_detail::is_equality_result_convertible_to_bool_v, bool> + operator==( + const expected &x, const expected &y + ) noexcept(expected_detail::is_nothrow_equality_result_convertible_to_bool_v) { if (x.has_value() != y.has_value()) { return false; } + else if (x.has_value()) + { + return true; + } else { - return x.has_value() || static_cast(x.error() == y.error()); + return x.error() == y.error(); } } #if ZEUS_EXPECTED_CPLUSPLUS < 202'002L template - [[nodiscard]] friend constexpr std::enable_if_t, bool> operator!=( - const expected &x, const expected &y - ) noexcept(noexcept(x == y)) + [[nodiscard]] friend constexpr std:: + enable_if_t && expected_detail::is_equality_result_convertible_to_bool_v, bool> operator!=( + const expected &x, const expected &y + ) noexcept(noexcept(x == y)) { return !(x == y); } #endif template - [[nodiscard]] friend constexpr bool operator==(const expected &x, const unexpected &e) noexcept(noexcept(x.error() == e.error())) + [[nodiscard]] friend constexpr std::enable_if_t, bool> operator==( + const expected &x, const unexpected &e + ) noexcept(expected_detail::is_nothrow_equality_result_convertible_to_bool_v) { if (x.has_value()) { @@ -2775,28 +2816,59 @@ class expected } else { - return static_cast(x.error() == e.error()); + return x.error() == e.error(); } } #if ZEUS_EXPECTED_CPLUSPLUS < 202'002L template - [[nodiscard]] friend constexpr bool operator!=(const expected &x, const unexpected &e) noexcept(noexcept(x == e)) + [[nodiscard]] friend constexpr std::enable_if_t, bool> operator!=( + const expected &x, const unexpected &e + ) noexcept(noexcept(x == e)) { return !(x == e); } - template - [[nodiscard]] friend constexpr bool operator==(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) - { - return x == e; - } - template - [[nodiscard]] friend constexpr bool operator!=(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) - { - return x != e; - } #endif }; +#if ZEUS_EXPECTED_CPLUSPLUS < 202'002L +// Deduce the expected operand to reject conversions exposed by MSVC's permissive C++17 hidden-friend lookup. +template +[[nodiscard]] constexpr std::enable_if_t< + !std::is_void_v && + !expected_detail::is_specialization_v && + expected_detail::is_equality_result_convertible_to_bool_v, + bool +> +operator==(const T2 &v, const expected &x) noexcept(noexcept(x == v)) +{ + return x == v; +} + +template +[[nodiscard]] constexpr std::enable_if_t< + !std::is_void_v && + !expected_detail::is_specialization_v && + expected_detail::is_equality_result_convertible_to_bool_v, + bool +> +operator!=(const T2 &v, const expected &x) noexcept(noexcept(x == v)) +{ + return x != v; +} + +template> * = nullptr> +[[nodiscard]] constexpr bool operator==(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) +{ + return x == e; +} + +template> * = nullptr> +[[nodiscard]] constexpr bool operator!=(const unexpected &e, const expected &x) noexcept(noexcept(x == e)) +{ + return x != e; +} +#endif + // standalone swap for void value type template && std::is_swappable_v> * = nullptr> constexpr void swap(expected &lhs, expected &rhs) noexcept(noexcept(lhs.swap(rhs))) diff --git a/tests/test_expected/CMakeLists.txt b/tests/test_expected/CMakeLists.txt index 2112537..3dc4efa 100644 --- a/tests/test_expected/CMakeLists.txt +++ b/tests/test_expected/CMakeLists.txt @@ -5,10 +5,13 @@ set(SOURCES monadic_tests.cpp noexcept_tests.cpp equality_tests.cpp + equality_noexcept_tests.cpp + p3379_tests.cpp lwg_3886_tests.cpp lwg_4031_tests.cpp lwg_4222_tests.cpp lwg_4025_tests.cpp + lwg_4366_tests.cpp ) find_package(Catch2 3 REQUIRED) diff --git a/tests/test_expected/equality_noexcept_tests.cpp b/tests/test_expected/equality_noexcept_tests.cpp new file mode 100644 index 0000000..2100429 --- /dev/null +++ b/tests/test_expected/equality_noexcept_tests.cpp @@ -0,0 +1,102 @@ +#include + +#include + +#include + +namespace +{ + +template +struct BooleanConversion +{ + bool value; + + constexpr operator bool() const noexcept(IsNothrow) { return value; } +}; + +struct ThrowingConversionLhs +{ +}; + +struct ThrowingConversionRhs +{ +}; + +BooleanConversion operator==(const ThrowingConversionLhs &, const ThrowingConversionRhs &) noexcept +{ + return {true}; +} + +struct NothrowConversionLhs +{ +}; + +struct NothrowConversionRhs +{ +}; + +BooleanConversion operator==(const NothrowConversionLhs &, const NothrowConversionRhs &) noexcept +{ + return {true}; +} + +} // namespace + +TEST_CASE("equality noexcept includes the implicit conversion to bool", "[noexcept][equality][LWG-4366]") +{ + using ObjectLhs = zeus::expected; + using ObjectRhs = zeus::expected; + using VoidLhs = zeus::expected; + using VoidRhs = zeus::expected; + using ObjectUnexpected = zeus::unexpected; + + // These are the five primary equality overloads. + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + + // These are explicit overloads in C++17 and rewritten equality candidates in C++20 and later. + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE_FALSE(noexcept(std::declval() != std::declval())); +} + +TEST_CASE("equality is noexcept when comparison and bool conversion are non-throwing", "[noexcept][equality]") +{ + using ObjectLhs = zeus::expected; + using ObjectRhs = zeus::expected; + using VoidLhs = zeus::expected; + using VoidRhs = zeus::expected; + using ObjectUnexpected = zeus::unexpected; + + // These are the five primary equality overloads. + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + + // These are explicit overloads in C++17 and rewritten equality candidates in C++20 and later. + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() == std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); + STATIC_REQUIRE(noexcept(std::declval() != std::declval())); +} diff --git a/tests/test_expected/lwg_4366_tests.cpp b/tests/test_expected/lwg_4366_tests.cpp new file mode 100644 index 0000000..f44d4d7 --- /dev/null +++ b/tests/test_expected/lwg_4366_tests.cpp @@ -0,0 +1,78 @@ +#include + +#include + +using namespace zeus; + +namespace +{ + +// P3379R0 accepts comparison results that are implicitly convertible to bool. +// LWG 4366 makes the function bodies honor that constraint instead of requiring +// an explicit conversion. +struct ImplicitBoolean +{ + bool value; + + constexpr operator bool() const noexcept { return value; } + explicit operator bool() = delete; +}; + +struct Lhs +{ + explicit constexpr Lhs(int v) + : value(v) + { + } + + int value; +}; + +struct Rhs +{ + explicit constexpr Rhs(int v) + : value(v) + { + } + + int value; +}; + +ImplicitBoolean operator==(const Lhs &lhs, const Rhs &rhs) +{ + return {lhs.value == rhs.value}; +} + +} // namespace + +TEST_CASE("LWG 4366 uses implicit conversion for expected-to-value comparison", "[LWG-4366][equality]") +{ + const expected value {Lhs {42}}; + + CHECK(value == Rhs {42}); + CHECK_FALSE(value == Rhs {7}); +} + +TEST_CASE("LWG 4366 uses implicit conversion for expected-to-unexpected comparison", "[LWG-4366][equality]") +{ + const expected error {unexpect, 42}; + + CHECK(error == zeus::unexpected {Rhs {42}}); + CHECK_FALSE(error == zeus::unexpected {Rhs {7}}); +} + +TEST_CASE("LWG 4366 uses implicit conversion for expected comparison", "[LWG-4366][equality]") +{ + const expected lhs {unexpect, 42}; + + CHECK(lhs == expected {unexpect, 42}); + CHECK_FALSE(lhs == expected {unexpect, 7}); +} + +TEST_CASE("LWG 4366 uses implicit conversion for expected-to-unexpected comparison", "[LWG-4366][equality]") +{ + const expected error {unexpect, 42}; + + CHECK(error == zeus::unexpected {Rhs {42}}); + CHECK_FALSE(error == zeus::unexpected {Rhs {7}}); +} diff --git a/tests/test_expected/p3379_tests.cpp b/tests/test_expected/p3379_tests.cpp new file mode 100644 index 0000000..dd15bc7 --- /dev/null +++ b/tests/test_expected/p3379_tests.cpp @@ -0,0 +1,243 @@ +#include + +#include + +#include +#include + +using namespace zeus; + +namespace +{ + +template +inline constexpr bool has_equal_to_v = false; + +template +inline constexpr bool has_equal_to_v() == std::declval())>> = true; + +template +inline constexpr bool has_not_equal_to_v = false; + +template +inline constexpr bool has_not_equal_to_v() != std::declval())>> = + true; + +struct NotBoolean +{ +}; + +struct NonBooleanComparable +{ +}; + +NotBoolean operator==(const NonBooleanComparable &, const NonBooleanComparable &) +{ + return {}; +} + +struct ImplicitBoolean +{ + bool value; + + constexpr operator bool() const noexcept { return value; } + explicit operator bool() = delete; +}; + +struct ImplicitlyBooleanComparable +{ +}; + +ImplicitBoolean operator==(const ImplicitlyBooleanComparable &, const ImplicitlyBooleanComparable &) +{ + return {true}; +} + +struct OtherError +{ +}; + +using ExpectedOperand = expected; + +struct ComparesWithExpected +{ +}; + +bool operator==(const ComparesWithExpected &, const ExpectedOperand &) +{ + return true; +} + +struct ValueError +{ +}; + +struct FallbackError +{ +}; + +struct UnexpectedOperandError +{ +}; + +struct ValueComparableToUnexpected +{ + bool equal; +}; + +bool operator==(const ValueComparableToUnexpected &value, const zeus::unexpected &) +{ + return value.equal; +} + +struct ComparableExpectedError +{ + int value; +}; + +struct ComparableUnexpectedError +{ + int value; +}; + +bool operator==(const ComparableExpectedError &lhs, const ComparableUnexpectedError &rhs) +{ + return lhs.value == rhs.value; +} + +struct AlsoComparableToUnexpected +{ +}; + +bool operator==(const AlsoComparableToUnexpected &, const zeus::unexpected &) +{ + return true; +} + +} // namespace + +TEST_CASE("P3379R0 constrains expected-to-expected comparison", "[P3379R0][equality]") +{ + using ValidComparison = expected; + using InvalidValueResult = expected; + using InvalidErrorResult = expected; + using ImplicitlyConvertible = expected; + + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE(has_equal_to_v); +} + +TEST_CASE("P3379R0 constrains both orders of heterogeneous expected comparisons", "[P3379R0][equality]") +{ + using ValidComparison = expected; + using InvalidValueResult = expected; + using InvalidErrorResult = expected; + + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 constrains expected-to-value comparison", "[P3379R0][equality]") +{ + using InvalidValueResult = expected; + + STATIC_REQUIRE(has_equal_to_v, int>); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 prevents an expected operand from using the value overload", "[P3379R0][equality]") +{ + using Lhs = expected; + + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 keeps nested expected comparisons valid", "[P3379R0][equality]") +{ + using Inner = expected; + using NestedValue = expected; + using NestedError = expected; + + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE(has_not_equal_to_v); + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE(has_not_equal_to_v); + STATIC_REQUIRE(has_equal_to_v>); + STATIC_REQUIRE(has_not_equal_to_v>); + + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE(has_not_equal_to_v); + STATIC_REQUIRE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 constrains expected-to-unexpected comparison", "[P3379R0][equality]") +{ + using InvalidErrorResult = expected; + using InvalidUnexpected = zeus::unexpected; + + STATIC_REQUIRE(has_equal_to_v, zeus::unexpected>); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 constrains expected comparisons", "[P3379R0][equality]") +{ + using ValidExpected = expected; + using InvalidExpected = expected; + using InvalidUnexpected = zeus::unexpected; + + STATIC_REQUIRE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); + + STATIC_REQUIRE_FALSE(has_equal_to_v); + STATIC_REQUIRE_FALSE(has_not_equal_to_v); +} + +TEST_CASE("P3379R0 leaves unexpected available to the value comparison overload", "[P3379R0][equality]") +{ + const zeus::unexpected operand {UnexpectedOperandError {}}; + const expected matching_value {ValueComparableToUnexpected {true}}; + const expected nonmatching_value {ValueComparableToUnexpected {false}}; + const expected error_value {unexpect, FallbackError {}}; + + CHECK(matching_value == operand); + CHECK(operand == matching_value); + CHECK_FALSE(matching_value != operand); + CHECK_FALSE(operand != matching_value); + CHECK_FALSE(nonmatching_value == operand); + CHECK_FALSE(error_value == operand); +} + +TEST_CASE("P3379R0 prefers the unexpected overload when its constraint is satisfied", "[P3379R0][equality]") +{ + const zeus::unexpected operand {ComparableUnexpectedError {42}}; + const expected value {AlsoComparableToUnexpected {}}; + const expected error {unexpect, ComparableExpectedError {42}}; + + CHECK_FALSE(value == operand); + CHECK_FALSE(operand == value); + CHECK(error == operand); + CHECK(operand == error); +}