From 14c70a678a76457562a1be33d1809463dd3bf6e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8ger=20Hanseg=C3=A5rd?= Date: Wed, 19 Feb 2025 12:12:29 +0100 Subject: [PATCH 1/2] Rename tl::unexpected::value() to tl::unexpected::error() Fixes #149 --- include/tl/expected.hpp | 52 ++++++++++++++++++++--------------------- tests/constructors.cpp | 20 ++++++++++++++++ tests/issues.cpp | 2 +- tests/observers.cpp | 12 ++++++++++ tests/relops.cpp | 25 ++++++++++++++++++++ 5 files changed, 84 insertions(+), 27 deletions(-) diff --git a/include/tl/expected.hpp b/include/tl/expected.hpp index 3c22c5c..9d3a5e1 100644 --- a/include/tl/expected.hpp +++ b/include/tl/expected.hpp @@ -165,10 +165,10 @@ template class unexpected { constexpr explicit unexpected(std::initializer_list l, Args &&...args) : m_val(l, std::forward(args)...) {} - constexpr const E &value() const & { return m_val; } - TL_EXPECTED_11_CONSTEXPR E &value() & { return m_val; } - TL_EXPECTED_11_CONSTEXPR E &&value() && { return std::move(m_val); } - constexpr const E &&value() const && { return std::move(m_val); } + constexpr const E &error() const & { return m_val; } + TL_EXPECTED_11_CONSTEXPR E &error() & { return m_val; } + TL_EXPECTED_11_CONSTEXPR E &&error() && { return std::move(m_val); } + constexpr const E &&error() const && { return std::move(m_val); } private: E m_val; @@ -180,27 +180,27 @@ template unexpected(E) -> unexpected; template constexpr bool operator==(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() == rhs.value(); + return lhs.error() == rhs.error(); } template constexpr bool operator!=(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() != rhs.value(); + return lhs.error() != rhs.error(); } template constexpr bool operator<(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() < rhs.value(); + return lhs.error() < rhs.error(); } template constexpr bool operator<=(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() <= rhs.value(); + return lhs.error() <= rhs.error(); } template constexpr bool operator>(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() > rhs.value(); + return lhs.error() > rhs.error(); } template constexpr bool operator>=(const unexpected &lhs, const unexpected &rhs) { - return lhs.value() >= rhs.value(); + return lhs.error() >= rhs.error(); } template @@ -1582,7 +1582,7 @@ class expected : private detail::expected_move_assign_base, detail::enable_if_t::value> * = nullptr> explicit constexpr expected(const unexpected &e) - : impl_base(unexpect, e.value()), + : impl_base(unexpect, e.error()), ctor_base(detail::default_constructor_tag{}) {} template < @@ -1591,7 +1591,7 @@ class expected : private detail::expected_move_assign_base, nullptr, detail::enable_if_t::value> * = nullptr> constexpr expected(unexpected const &e) - : impl_base(unexpect, e.value()), + : impl_base(unexpect, e.error()), ctor_base(detail::default_constructor_tag{}) {} template < @@ -1600,7 +1600,7 @@ class expected : private detail::expected_move_assign_base, detail::enable_if_t::value> * = nullptr> explicit constexpr expected(unexpected &&e) noexcept( std::is_nothrow_constructible::value) - : impl_base(unexpect, std::move(e.value())), + : impl_base(unexpect, std::move(e.error())), ctor_base(detail::default_constructor_tag{}) {} template < @@ -1609,7 +1609,7 @@ class expected : private detail::expected_move_assign_base, detail::enable_if_t::value> * = nullptr> constexpr expected(unexpected &&e) noexcept( std::is_nothrow_constructible::value) - : impl_base(unexpect, std::move(e.value())), + : impl_base(unexpect, std::move(e.error())), ctor_base(detail::default_constructor_tag{}) {} template , detail::enable_if_t::value> * = nullptr> TL_EXPECTED_11_CONSTEXPR const U &value() const & { if (!has_value()) - detail::throw_exception(bad_expected_access(err().value())); + detail::throw_exception(bad_expected_access(err().error())); return val(); } template ::value> * = nullptr> TL_EXPECTED_11_CONSTEXPR U &value() & { if (!has_value()) - detail::throw_exception(bad_expected_access(err().value())); + detail::throw_exception(bad_expected_access(err().error())); return val(); } template ::value> * = nullptr> TL_EXPECTED_11_CONSTEXPR const U &&value() const && { if (!has_value()) - detail::throw_exception(bad_expected_access(std::move(err()).value())); + detail::throw_exception(bad_expected_access(std::move(err()).error())); return std::move(val()); } template ::value> * = nullptr> TL_EXPECTED_11_CONSTEXPR U &&value() && { if (!has_value()) - detail::throw_exception(bad_expected_access(std::move(err()).value())); + detail::throw_exception(bad_expected_access(std::move(err()).error())); return std::move(val()); } constexpr const E &error() const & { TL_ASSERT(!has_value()); - return err().value(); + return err().error(); } TL_EXPECTED_11_CONSTEXPR E &error() & { TL_ASSERT(!has_value()); - return err().value(); + return err().error(); } constexpr const E &&error() const && { TL_ASSERT(!has_value()); - return std::move(err().value()); + return std::move(err().error()); } TL_EXPECTED_11_CONSTEXPR E &&error() && { TL_ASSERT(!has_value()); - return std::move(err().value()); + return std::move(err().error()); } template constexpr T value_or(U &&v) const & { @@ -2446,19 +2446,19 @@ constexpr bool operator!=(const U &v, const expected &x) { template constexpr bool operator==(const expected &x, const unexpected &e) { - return x.has_value() ? false : x.error() == e.value(); + return x.has_value() ? false : x.error() == e.error(); } template constexpr bool operator==(const unexpected &e, const expected &x) { - return x.has_value() ? false : x.error() == e.value(); + return x.has_value() ? false : x.error() == e.error(); } template constexpr bool operator!=(const expected &x, const unexpected &e) { - return x.has_value() ? true : x.error() != e.value(); + return x.has_value() ? true : x.error() != e.error(); } template constexpr bool operator!=(const unexpected &e, const expected &x) { - return x.has_value() ? true : x.error() != e.value(); + return x.has_value() ? true : x.error() != e.error(); } template u('s'); + tl::expected e(u); + REQUIRE(e.error() == 's'); + } + + { + struct value { + constexpr explicit value(char v) : val(v) {} + char val; + }; + + constexpr tl::unexpected u('s'); + tl::expected e1(u); + REQUIRE(e1.error().val == 's'); + + tl::expected e2(tl::unexpected('s')); + REQUIRE(e2.error().val == 's'); + } } diff --git a/tests/issues.cpp b/tests/issues.cpp index f929099..9efe14b 100644 --- a/tests/issues.cpp +++ b/tests/issues.cpp @@ -166,7 +166,7 @@ TEST_CASE("Issue 122", "[issues.122]") { #ifdef __cpp_deduction_guides TEST_CASE("Issue 89", "[issues.89]") { auto s = tl::unexpected("Some string"); - REQUIRE(s.value() == std::string("Some string")); + REQUIRE(s.error() == std::string("Some string")); } #endif diff --git a/tests/observers.cpp b/tests/observers.cpp index 5d8473c..cff3e9f 100644 --- a/tests/observers.cpp +++ b/tests/observers.cpp @@ -34,3 +34,15 @@ TEST_CASE("Observers", "[observers]") { REQUIRE(o4->been_moved); REQUIRE(!o5.been_moved); } + +#ifdef TL_EXPECTED_EXCEPTIONS_ENABLED +TEST_CASE("Observers invalid access", "[observers]") { + tl::expected err(tl::make_unexpected('!')); + + REQUIRE_THROWS_AS(err.value(), tl::bad_expected_access); + REQUIRE_THROWS_AS(std::as_const(err).value(), tl::bad_expected_access); + REQUIRE_THROWS_AS(std::move(std::as_const(err)).value(), + tl::bad_expected_access); + REQUIRE_THROWS_AS(std::move(err).value(), tl::bad_expected_access); +} +#endif diff --git a/tests/relops.cpp b/tests/relops.cpp index 99dee5f..b873b6a 100644 --- a/tests/relops.cpp +++ b/tests/relops.cpp @@ -15,3 +15,28 @@ TEST_CASE("Relational operators", "[relops]") { REQUIRE(o6 == o6); } + +TEST_CASE("Relational operators unexpected", "[relops]") { + tl::unexpected zero(0); + tl::unexpected one(1); + + REQUIRE(one == one); + REQUIRE(one != zero); + REQUIRE(zero < one); + REQUIRE(zero <= one); + REQUIRE(one <= one); + REQUIRE(one > zero); + REQUIRE(one >= zero); + REQUIRE(one >= one); +} + +TEST_CASE("Relational operators expected vs unexpected", "[relops]") { + tl::expected ezero(tl::unexpect, 0); + tl::unexpected uzero(0); + tl::unexpected uone(1); + + REQUIRE(ezero == uzero); + REQUIRE(ezero != uone); + REQUIRE(uzero == ezero); + REQUIRE(uone != ezero); +} From c6064c91aa84412d07a4f3948d19496e0f294cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8ger=20Hanseg=C3=A5rd?= Date: Wed, 19 Feb 2025 14:05:45 +0100 Subject: [PATCH 2/2] Require in_place_t with variadic unexpected ctors This makes unexpected ctors more consistent with the C++ standard --- include/tl/expected.hpp | 31 +++++----- tests/constructors.cpp | 123 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 14 deletions(-) diff --git a/include/tl/expected.hpp b/include/tl/expected.hpp index 9d3a5e1..83920bd 100644 --- a/include/tl/expected.hpp +++ b/include/tl/expected.hpp @@ -156,13 +156,15 @@ template class unexpected { template ::value>::type * = nullptr> - constexpr explicit unexpected(Args &&...args) + constexpr explicit unexpected(in_place_t, Args &&...args) : m_val(std::forward(args)...) {} + template < class U, class... Args, typename std::enable_if &, Args &&...>::value>::type * = nullptr> - constexpr explicit unexpected(std::initializer_list l, Args &&...args) + constexpr explicit unexpected(in_place_t, std::initializer_list l, + Args &&...args) : m_val(l, std::forward(args)...) {} constexpr const E &error() const & { return m_val; } @@ -471,7 +473,7 @@ struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), m_has_val(false) {} ~expected_storage_base() { if (m_has_val) { @@ -518,7 +520,7 @@ template struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template struct expected_storage_base { constexpr explicit expected_storage_base(unexpect_t, std::initializer_list il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), m_has_val(false) {} expected_storage_base(const expected_storage_base &) = default; expected_storage_base(expected_storage_base &&) = default; @@ -563,7 +565,7 @@ template struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template struct expected_storage_base { constexpr explicit expected_storage_base(unexpect_t, std::initializer_list il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), m_has_val(false) {} expected_storage_base(const expected_storage_base &) = default; expected_storage_base(expected_storage_base &&) = default; @@ -612,7 +614,7 @@ template struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template struct expected_storage_base { constexpr explicit expected_storage_base(unexpect_t, std::initializer_list il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), + m_has_val(false) {} expected_storage_base(const expected_storage_base &) = default; expected_storage_base(expected_storage_base &&) = default; @@ -656,7 +659,7 @@ template struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template struct expected_storage_base { constexpr explicit expected_storage_base(unexpect_t, std::initializer_list il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), m_has_val(false) {} expected_storage_base(const expected_storage_base &) = default; expected_storage_base(expected_storage_base &&) = default; @@ -690,7 +693,7 @@ template struct expected_storage_base { detail::enable_if_t::value> * = nullptr> constexpr explicit expected_storage_base(unexpect_t, Args &&...args) - : m_unexpect(std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, std::forward(args)...), m_has_val(false) {} template struct expected_storage_base { constexpr explicit expected_storage_base(unexpect_t, std::initializer_list il, Args &&...args) - : m_unexpect(il, std::forward(args)...), m_has_val(false) {} + : m_unexpect(in_place, il, std::forward(args)...), m_has_val(false) {} expected_storage_base(const expected_storage_base &) = default; expected_storage_base(expected_storage_base &&) = default; diff --git a/tests/constructors.cpp b/tests/constructors.cpp index 10b96de..f24a46a 100644 --- a/tests/constructors.cpp +++ b/tests/constructors.cpp @@ -3,6 +3,7 @@ #include #include +#include #include struct takes_init_and_variadic { @@ -13,6 +14,27 @@ struct takes_init_and_variadic { : v(l), t(std::forward(args)...) {} }; +struct trivial_type { + int x; + int y; + trivial_type(int _x, int _y) : x{_x}, y{_y} {} + trivial_type(std::initializer_list list) { + auto it = list.begin(); + x = *it; + ++it; + y = *it; + } +}; + +struct takes_init_and_variadic_trivial { + trivial_type p; + std::tuple t; + template + takes_init_and_variadic_trivial(std::initializer_list l, + Args &&...args) + : p(l), t(std::forward(args)...) {} +}; + TEST_CASE("Constructors", "[constructors]") { { tl::expected e; @@ -38,6 +60,12 @@ TEST_CASE("Constructors", "[constructors]") { REQUIRE(e == 42); } + { + tl::expected e(tl::in_place); + REQUIRE(e); + REQUIRE(e == 0); + } + { tl::expected,int> e (tl::in_place, {0,1}); REQUIRE(e); @@ -52,6 +80,40 @@ TEST_CASE("Constructors", "[constructors]") { REQUIRE(std::get<1>(*e) == 1); } + { + tl::expected> e(tl::unexpect, 0, 1); + REQUIRE(!e); + REQUIRE(std::get<0>(e.error()) == 0); + REQUIRE(std::get<1>(e.error()) == 1); + } + + { + tl::expected> e(tl::unexpect, 0, 1); + REQUIRE(!e); + REQUIRE(std::get<0>(e.error()) == 0); + REQUIRE(std::get<1>(e.error()) == 1); + } + { + tl::expected> e(tl::unexpect, 2, 1); + REQUIRE(!e); + REQUIRE(e.error()[0] == 1); + REQUIRE(e.error()[1] == 1); + } + { + tl::expected, std::vector> e(tl::unexpect, 2, 1); + REQUIRE(!e); + REQUIRE(e.error()[0] == 1); + REQUIRE(e.error()[1] == 1); + } + { + tl::expected, takes_init_and_variadic> e(tl::unexpect, + {0, 1}, 2, 3); + REQUIRE(!e); + REQUIRE(e.error().v[0] == 0); + REQUIRE(e.error().v[1] == 1); + REQUIRE(std::get<0>(e.error().t) == 2); + REQUIRE(std::get<1>(e.error().t) == 3); + } { tl::expected e (tl::in_place, {0,1}, 2, 3); REQUIRE(e); @@ -60,6 +122,59 @@ TEST_CASE("Constructors", "[constructors]") { REQUIRE(std::get<0>(e->t) == 2); REQUIRE(std::get<1>(e->t) == 3); } + { + tl::expected e(tl::unexpect, {0, 1}, 2, 3); + REQUIRE(!e); + REQUIRE(e.error().v[0] == 0); + REQUIRE(e.error().v[1] == 1); + REQUIRE(std::get<0>(e.error().t) == 2); + REQUIRE(std::get<1>(e.error().t) == 3); + } + { + tl::expected e(tl::unexpect, {0, 1}, 2, 3); + REQUIRE(!e); + REQUIRE(e.error().p.x == 0); + REQUIRE(e.error().p.y == 1); + REQUIRE(std::get<0>(e.error().t) == 2); + REQUIRE(std::get<1>(e.error().t) == 3); + } + { + tl::expected e(tl::unexpect, + {0, 1}, 2, 3); + REQUIRE(!e); + REQUIRE(e.error().p.x == 0); + REQUIRE(e.error().p.y == 1); + REQUIRE(std::get<0>(e.error().t) == 2); + REQUIRE(std::get<1>(e.error().t) == 3); + } + { + tl::expected> e(tl::unexpect, 2, 1); + REQUIRE(!e); + REQUIRE(e.error()[0] == 1); + REQUIRE(e.error()[1] == 1); + } + { + tl::expected, trivial_type> e(tl::unexpect, 1, 2); + REQUIRE(!e); + REQUIRE(e.error().x == 1); + REQUIRE(e.error().y == 2); + } + { + tl::expected, takes_init_and_variadic_trivial> e(tl::unexpect, {1, 2}, 3, 4); + REQUIRE(!e); + REQUIRE(e.error().p.x == 1); + REQUIRE(e.error().p.y == 2); + REQUIRE(std::get<0>(e.error().t) == 3); + REQUIRE(std::get<1>(e.error().t) == 4); + } + { + tl::expected e(tl::unexpect, {0, 1}, 2, 3); + REQUIRE(!e); + REQUIRE(e.error().v[0] == 0); + REQUIRE(e.error().v[1] == 1); + REQUIRE(std::get<0>(e.error().t) == 2); + REQUIRE(std::get<1>(e.error().t) == 3); + } { tl::expected e; @@ -152,3 +267,11 @@ TEST_CASE("Constructors", "[constructors]") { REQUIRE(e2.error().val == 's'); } } + +TEST_CASE("Unexpected constructors", "[constructors]") { + REQUIRE(tl::unexpected(1).error() == 1); + REQUIRE(tl::unexpected(tl::in_place).error() == 0); + REQUIRE(tl::unexpected(tl::in_place, 1).error() == 1); + REQUIRE(tl::unexpected>(tl::in_place, {1, 2, 3}).error() == + std::vector{1, 2, 3}); +}