From a884b2add049e2ccef8bf48d2c3c3fd145c70836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= Date: Tue, 31 Dec 2024 13:31:14 +0100 Subject: [PATCH 1/2] Set 'engaged' flag on assignment correctly Also, refactor some other places to make use of the 'construct' helper that handles the flag. --- include/beman/optional26/optional.hpp | 19 +++++++------- src/beman/optional26/tests/optional.t.cpp | 32 ++++++++++++++++++++++- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/include/beman/optional26/optional.hpp b/include/beman/optional26/optional.hpp index 54ebd68..d2a4cdf 100644 --- a/include/beman/optional26/optional.hpp +++ b/include/beman/optional26/optional.hpp @@ -424,8 +424,7 @@ inline constexpr optional::optional(const optional& rhs) requires std::is_copy_constructible_v && (!std::is_trivially_copy_constructible_v) { if (rhs.has_value()) { - std::construct_at(std::addressof(value_), rhs.value_); - engaged_ = true; + construct(rhs.value_); } } @@ -434,8 +433,7 @@ inline constexpr optional::optional(optional&& rhs) noexcept(std::is_nothrow_ requires std::is_move_constructible_v && (!std::is_trivially_move_constructible_v) { if (rhs.has_value()) { - std::construct_at(std::addressof(value_), std::move(rhs.value())); - engaged_ = true; + construct(std::move(rhs.value_)); } } @@ -466,8 +464,7 @@ inline constexpr optional::optional(const optional& rhs) requires detail::enable_from_other && std::is_convertible_v { if (rhs.has_value()) { - std::construct_at(std::addressof(value_), rhs.value()); - engaged_ = true; + construct(*rhs); } } @@ -523,8 +520,9 @@ inline constexpr optional& optional::operator=(const optional& rhs) reset(); else if (has_value()) value_ = rhs.value_; - else - std::construct_at(std::addressof(value_), rhs.value_); + else { + construct(rhs.value_); + } return *this; } @@ -538,8 +536,9 @@ optional::operator=(optional&& rhs) noexcept(std::is_nothrow_move_construc reset(); else if (has_value()) value_ = std::move(rhs.value_); - else - std::construct_at(std::addressof(value_), std::move(rhs.value_)); + else { + construct(std::move(rhs.value_)); + } return *this; } diff --git a/src/beman/optional26/tests/optional.t.cpp b/src/beman/optional26/tests/optional.t.cpp index ca88391..d0592fc 100644 --- a/src/beman/optional26/tests/optional.t.cpp +++ b/src/beman/optional26/tests/optional.t.cpp @@ -208,8 +208,38 @@ TEST(OptionalTest, AssignmentValue) { */ beman::optional26::optional o5{5}; beman::optional26::optional o6; + + // Copy into empty optional. o6 = o5; - EXPECT_TRUE(o5->i_ == 5); + EXPECT_TRUE(o6); + EXPECT_TRUE(o6->i_ == 5); + + // Move into empty optional. + o6.reset(); + o6 = std::move(o5); + EXPECT_TRUE(o6); + EXPECT_TRUE(o6->i_ == 5); + + // Copy into engaged optional. + beman::optional26::optional o7{7}; + o6 = o7; + EXPECT_TRUE(o6); + EXPECT_TRUE(o6->i_ == 7); + + // Move into engaged optional. + beman::optional26::optional o8{8}; + o6 = std::move(o8); + EXPECT_TRUE(o6); + EXPECT_TRUE(o6->i_ == 8); + + // Copy from empty into engaged optional. + o5.reset(); + o7 = o5; + EXPECT_FALSE(o7); + + // Move from empty into engaged optional. + o8 = std::move(o5); + EXPECT_FALSE(o8); } TEST(OptionalTest, Triviality) { From 9a60702991122e899429be2b727799a5ac6493dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= Date: Tue, 31 Dec 2024 13:36:01 +0100 Subject: [PATCH 2/2] Remove braces --- include/beman/optional26/optional.hpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/beman/optional26/optional.hpp b/include/beman/optional26/optional.hpp index d2a4cdf..6fe745f 100644 --- a/include/beman/optional26/optional.hpp +++ b/include/beman/optional26/optional.hpp @@ -520,9 +520,8 @@ inline constexpr optional& optional::operator=(const optional& rhs) reset(); else if (has_value()) value_ = rhs.value_; - else { + else construct(rhs.value_); - } return *this; } @@ -536,9 +535,8 @@ optional::operator=(optional&& rhs) noexcept(std::is_nothrow_move_construc reset(); else if (has_value()) value_ = std::move(rhs.value_); - else { + else construct(std::move(rhs.value_)); - } return *this; }