diff --git a/README.md b/README.md index 375c414..e983eee 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ Implemented LWG Issues: - [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 +- [x] [LWG-3891](https://wg21.link/lwg3891) LWG 3870 breaks `std::expected` Enhancements: diff --git a/include/zeus/expected.hpp b/include/zeus/expected.hpp index 686037a..3ab4b0f 100644 --- a/include/zeus/expected.hpp +++ b/include/zeus/expected.hpp @@ -597,9 +597,11 @@ struct storage_base union { - T m_val; - E m_unexpect; - char m_dummy; + // LWG-3891: store remove_cv_t, construct/destroy it as such, and + // access it through a cv-qualified glvalue elsewhere. + std::remove_cv_t m_val; + E m_unexpect; + char m_dummy; }; bool m_has_val; }; @@ -710,9 +712,11 @@ struct storage_base union { - T m_val; - E m_unexpect; - char m_dummy; + // LWG-3891: store remove_cv_t, construct/destroy it as such, and + // access it through a cv-qualified glvalue elsewhere. + std::remove_cv_t m_val; + E m_unexpect; + char m_dummy; }; bool m_has_val; }; @@ -1299,16 +1303,16 @@ class expected static_assert(expected_detail::is_value_type_valid_v); static_assert(expected_detail::is_error_type_valid_v); - constexpr T *valptr() noexcept { return std::addressof(this->m_val); } - constexpr const T *valptr() const noexcept { return std::addressof(this->m_val); } - constexpr E *errptr() noexcept { return std::addressof(this->m_unexpect); } - constexpr const E *errptr() const noexcept { return std::addressof(this->m_unexpect); } + constexpr std::remove_cv_t *valptr() noexcept { return std::addressof(this->m_val); } + constexpr const std::remove_cv_t *valptr() const noexcept { return std::addressof(this->m_val); } + constexpr E *errptr() noexcept { return std::addressof(this->m_unexpect); } + constexpr const E *errptr() const noexcept { return std::addressof(this->m_unexpect); } - constexpr T &val() noexcept { return this->m_val; } - constexpr E &err() noexcept { return this->m_unexpect; } + constexpr std::remove_cv_t &val() noexcept { return this->m_val; } + constexpr E &err() noexcept { return this->m_unexpect; } - constexpr const T &val() const noexcept { return this->m_val; } - constexpr const E &err() const noexcept { return this->m_unexpect; } + constexpr const std::remove_cv_t &val() const noexcept { return this->m_val; } + constexpr const E &err() const noexcept { return this->m_unexpect; } using impl_base = expected_detail::move_assign_base; using ctor_base = expected_detail::default_ctor_base; @@ -1711,13 +1715,14 @@ class expected } else { - T tmp(std::move(this->m_val)); + // LWG-3891: the internal backup is considered cv-unqualified. + std::remove_cv_t tmp(std::move(this->m_val)); if constexpr (!std::is_trivially_destructible_v) { this->m_val.~T(); } - expected_detail::ReinitGuard guard {std::addressof(this->m_val), std::addressof(tmp)}; + expected_detail::ReinitGuard> guard {std::addressof(this->m_val), std::addressof(tmp)}; expected_detail::construct_at(std::addressof(this->error()), std::move(rhs.error())); guard._target = nullptr; diff --git a/tests/test_expected/CMakeLists.txt b/tests/test_expected/CMakeLists.txt index c72a853..18dc153 100644 --- a/tests/test_expected/CMakeLists.txt +++ b/tests/test_expected/CMakeLists.txt @@ -13,6 +13,7 @@ set(SOURCES unexpected_equality_tests.cpp p3379_tests.cpp lwg_3886_tests.cpp + lwg_3891_tests.cpp lwg_4031_tests.cpp lwg_4222_tests.cpp lwg_4025_tests.cpp diff --git a/tests/test_expected/lwg_3891_tests.cpp b/tests/test_expected/lwg_3891_tests.cpp new file mode 100644 index 0000000..d515109 --- /dev/null +++ b/tests/test_expected/lwg_3891_tests.cpp @@ -0,0 +1,182 @@ +#include + +#include + +#include +#include +#include + +using namespace zeus; + +namespace +{ + +struct ThrowingMoveError +{ + int value = 0; + + ThrowingMoveError() = default; + explicit ThrowingMoveError(int v) + : value(v) + { + } + ThrowingMoveError(ThrowingMoveError const &) = default; + ThrowingMoveError(ThrowingMoveError &&other) noexcept(false) + : value(other.value) + { + } + ThrowingMoveError &operator=(ThrowingMoveError const &) = default; + ThrowingMoveError &operator=(ThrowingMoveError &&other) noexcept(false) + { + value = other.value; + return *this; + } + + friend void swap(ThrowingMoveError &, ThrowingMoveError &) noexcept {} +}; + +static_assert(!std::is_nothrow_move_constructible_v); + +} // namespace + +SCENARIO("expected stores remove_cv_t", "[LWG-3891]") +{ + GIVEN("a cv-qualified scalar value type") + { + static_assert(std::is_swappable_v); + static_assert(std::is_copy_assignable_v); + + WHEN("constructing and reading the value") + { + expected value {1}; + + CHECK(value.value() == 1); + CHECK(*value == 1); + } + + WHEN("assigning between expected objects") + { + expected lhs {1}; + expected rhs {2}; + + lhs = rhs; + CHECK(lhs.value() == 2); + + lhs = std::move(rhs); + CHECK(lhs.value() == 2); + } + + WHEN("assigning a value") + { + expected e {zeus::unexpect, 0}; + + e = 3; + REQUIRE(e.has_value()); + CHECK(e.value() == 3); + } + + WHEN("transitioning between the value and the error state") + { + expected e {1}; + + e = zeus::unexpected {4}; + REQUIRE_FALSE(e.has_value()); + CHECK(e.error() == 4); + + e = 5; + REQUIRE(e.has_value()); + CHECK(e.value() == 5); + } + + WHEN("swapping") + { + expected lhs {1}; + expected rhs {2}; + + lhs.swap(rhs); + CHECK(lhs.value() == 2); + CHECK(rhs.value() == 1); + + expected err {zeus::unexpect, 9}; + lhs.swap(err); + REQUIRE_FALSE(lhs.has_value()); + CHECK(lhs.error() == 9); + REQUIRE(err.has_value()); + CHECK(err.value() == 2); + } + + WHEN("emplacing") + { + expected e {1}; + + CHECK(e.emplace(7) == 7); + CHECK(e.value() == 7); + } + } + + GIVEN("a cv-qualified value type with a non-trivial error type") + { + WHEN("assigning and swapping") + { + expected v {1}; + expected e {zeus::unexpect, "boom"}; + + v = e; + REQUIRE_FALSE(v.has_value()); + CHECK(v.error() == "boom"); + + v = 2; + REQUIRE(v.has_value()); + CHECK(v.value() == 2); + + v.swap(e); + REQUIRE_FALSE(v.has_value()); + CHECK(v.error() == "boom"); + REQUIRE(e.has_value()); + CHECK(e.value() == 2); + } + } + + GIVEN("a cv-qualified value type whose error type is not nothrow move constructible") + { + WHEN("swapping mixed states") + { + expected v {1}; + expected e {zeus::unexpect, ThrowingMoveError {9}}; + + v.swap(e); + REQUIRE_FALSE(v.has_value()); + CHECK(v.error().value == 9); + REQUIRE(e.has_value()); + CHECK(e.value() == 1); + } + } + + GIVEN("a const-qualified value type") + { + static_assert(!std::is_copy_assignable_v>); + static_assert(!std::is_move_assignable_v>); + static_assert(!std::is_swappable_v>); + + WHEN("constructing, reading and emplacing") + { + expected e {1}; + + const int &r = e.value(); + CHECK(r == 1); + + CHECK(e.emplace(2) == 2); + CHECK(e.value() == 2); + } + + WHEN("accessing through a cv-qualified glvalue") + { + expected e {3}; + + const volatile int &r = e.value(); + CHECK(r == 3); + CHECK(*e == 3); + CHECK(e.operator->() != nullptr); + } + } +}