Skip to content

Commit bb60b34

Browse files
steve-downeysteve-downey
authored
Merge pull request #93 from jiixyj/shallow-value-category-on-move
When moving a 'optional<T&>', don't move the remote value
2 parents 5460182 + e5fab32 commit bb60b34

File tree

3 files changed

+197
-12
lines changed

3 files changed

+197
-12
lines changed

‎include/beman/optional26/optional.hpp‎

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -276,19 +276,29 @@ class optional {
276276

277277
template <class U>
278278
constexpr explicit(!std::is_convertible_v<U, T>) optional(const optional<U>& rhs)
279-
requires detail::enable_from_other<T, U, const U&> && std::is_convertible_v<const U&, T>;
279+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, const U&> &&
280+
std::is_convertible_v<const U&, T>);
280281

281282
template <class U>
282283
constexpr explicit(!std::is_convertible_v<U, T>) optional(const optional<U>& rhs)
283-
requires detail::enable_from_other<T, U, const U&> && (!std::is_convertible_v<const U&, T>);
284+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, const U&> &&
285+
!std::is_convertible_v<const U&, T>);
284286

285287
template <class U>
286288
constexpr explicit(!std::is_convertible_v<U, T>) optional(optional<U>&& rhs)
287-
requires detail::enable_from_other<T, U, U&&> && std::is_convertible_v<U&&, T>;
289+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&> && std::is_convertible_v<U &&, T>);
288290

289291
template <class U>
290292
constexpr explicit(!std::is_convertible_v<U, T>) optional(optional<U>&& rhs)
291-
requires detail::enable_from_other<T, U, U&&> && (!std::is_convertible_v<U &&, T>);
293+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&> && !std::is_convertible_v<U &&, T>);
294+
295+
template <class U>
296+
constexpr explicit(!std::is_convertible_v<U&, T>) optional(const optional<U&>& rhs)
297+
requires(detail::enable_from_other<T, U&, U&> && std::is_convertible_v<U&, T>);
298+
299+
template <class U>
300+
constexpr explicit(!std::is_convertible_v<U&, T>) optional(const optional<U&>& rhs)
301+
requires(detail::enable_from_other<T, U&, U&> && !std::is_convertible_v<U&, T>);
292302

293303
// \ref{optional.dtor}, destructor
294304
constexpr ~optional()
@@ -325,11 +335,15 @@ class optional {
325335

326336
template <class U>
327337
constexpr optional& operator=(const optional<U>& rhs)
328-
requires detail::enable_assign_from_other<T, U, const U&>;
338+
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, const U&>);
329339

330340
template <class U>
331341
constexpr optional& operator=(optional<U>&& rhs)
332-
requires detail::enable_assign_from_other<T, U, U>;
342+
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>);
343+
344+
template <class U>
345+
constexpr optional& operator=(const optional<U&>& rhs)
346+
requires(detail::enable_assign_from_other<T, U&, U&>);
333347

334348
template <class... Args>
335349
constexpr T& emplace(Args&&... args);
@@ -463,7 +477,8 @@ inline constexpr optional<T>::optional(U&& u)
463477
template <class T>
464478
template <class U>
465479
inline constexpr optional<T>::optional(const optional<U>& rhs)
466-
requires detail::enable_from_other<T, U, const U&> && std::is_convertible_v<const U&, T>
480+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, const U&> &&
481+
std::is_convertible_v<const U&, T>)
467482
{
468483
if (rhs.has_value()) {
469484
construct(*rhs);
@@ -473,7 +488,8 @@ inline constexpr optional<T>::optional(const optional<U>& rhs)
473488
template <class T>
474489
template <class U>
475490
inline constexpr optional<T>::optional(const optional<U>& rhs)
476-
requires detail::enable_from_other<T, U, const U&> && (!std::is_convertible_v<const U&, T>)
491+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, const U&> &&
492+
!std::is_convertible_v<const U&, T>)
477493
{
478494
if (rhs.has_value()) {
479495
construct(*rhs);
@@ -484,7 +500,7 @@ inline constexpr optional<T>::optional(const optional<U>& rhs)
484500
template <class T>
485501
template <class U>
486502
inline constexpr optional<T>::optional(optional<U>&& rhs)
487-
requires detail::enable_from_other<T, U, U&&> && std::is_convertible_v<U&&, T>
503+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&> && std::is_convertible_v<U &&, T>)
488504
{
489505
if (rhs.has_value()) {
490506
construct(std::move(*rhs));
@@ -494,13 +510,33 @@ inline constexpr optional<T>::optional(optional<U>&& rhs)
494510
template <class T>
495511
template <class U>
496512
inline constexpr optional<T>::optional(optional<U>&& rhs)
497-
requires detail::enable_from_other<T, U, U&&> && (!std::is_convertible_v<U &&, T>)
513+
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&> && !std::is_convertible_v<U &&, T>)
498514
{
499515
if (rhs.has_value()) {
500516
construct(std::move(*rhs));
501517
}
502518
}
503519

520+
template <class T>
521+
template <class U>
522+
inline constexpr optional<T>::optional(const optional<U&>& rhs)
523+
requires(detail::enable_from_other<T, U&, U&> && std::is_convertible_v<U&, T>)
524+
{
525+
if (rhs.has_value()) {
526+
construct(*rhs);
527+
}
528+
}
529+
530+
template <class T>
531+
template <class U>
532+
inline constexpr optional<T>::optional(const optional<U&>& rhs)
533+
requires(detail::enable_from_other<T, U&, U&> && !std::is_convertible_v<U&, T>)
534+
{
535+
if (rhs.has_value()) {
536+
construct(*rhs);
537+
}
538+
}
539+
504540
// 22.5.3.3 Destructor[optional.dtor]
505541

506542
template <class T>
@@ -571,7 +607,7 @@ inline constexpr optional<T>& optional<T>::operator=(U&& u)
571607
template <class T>
572608
template <class U>
573609
inline constexpr optional<T>& optional<T>::operator=(const optional<U>& rhs)
574-
requires detail::enable_assign_from_other<T, U, const U&>
610+
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, const U&>)
575611
{
576612
if (has_value()) {
577613
if (rhs.has_value()) {
@@ -595,7 +631,7 @@ inline constexpr optional<T>& optional<T>::operator=(const optional<U>& rhs)
595631
template <class T>
596632
template <class U>
597633
inline constexpr optional<T>& optional<T>::operator=(optional<U>&& rhs)
598-
requires detail::enable_assign_from_other<T, U, U>
634+
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>)
599635
{
600636
if (has_value()) {
601637
if (rhs.has_value()) {
@@ -612,6 +648,26 @@ inline constexpr optional<T>& optional<T>::operator=(optional<U>&& rhs)
612648
return *this;
613649
}
614650

651+
template <class T>
652+
template <class U>
653+
inline constexpr optional<T>& optional<T>::operator=(const optional<U&>& rhs)
654+
requires(detail::enable_assign_from_other<T, U&, U&>)
655+
{
656+
if (has_value()) {
657+
if (rhs.has_value()) {
658+
value_ = *rhs;
659+
} else {
660+
hard_reset();
661+
}
662+
}
663+
664+
else if (rhs.has_value()) {
665+
construct(*rhs);
666+
}
667+
668+
return *this;
669+
}
670+
615671
/// Constructs the value in-place, destroying the current one if there is
616672
/// one.
617673
template <class T>

‎src/beman/optional26/tests/optional.t.cpp‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,3 +914,94 @@ TEST(OptionalTest, CanHoldValueOfImmovableType) {
914914
beman::optional26::optional<immovable> o2 = beman::optional26::nullopt;
915915
EXPECT_FALSE(o2);
916916
}
917+
918+
// Moving an `optional<T&>` should not move the remote value.
919+
TEST(OptionalTest, OptionalFromOptionalRef) {
920+
using beman::optional26::tests::copyable_from_non_const_lvalue_only;
921+
922+
copyable_from_non_const_lvalue_only cm;
923+
924+
beman::optional26::optional<copyable_from_non_const_lvalue_only&> o1 = cm;
925+
ASSERT_TRUE(o1);
926+
927+
{
928+
beman::optional26::optional<copyable_from_non_const_lvalue_only> o2 = o1;
929+
ASSERT_TRUE(o2);
930+
}
931+
932+
beman::optional26::optional<copyable_from_non_const_lvalue_only> o2 = std::move(o1);
933+
ASSERT_TRUE(o2);
934+
935+
o2 = o1;
936+
ASSERT_TRUE(o2);
937+
938+
o2 = std::move(o1);
939+
ASSERT_TRUE(o2);
940+
941+
o2.reset();
942+
o2 = o1;
943+
ASSERT_TRUE(o2);
944+
945+
o2.reset();
946+
o2 = std::move(o1);
947+
ASSERT_TRUE(o2);
948+
}
949+
950+
TEST(OptionalTest, OptionalFromOptionalRefExplicit) {
951+
using beman::optional26::tests::copyable_from_non_const_lvalue_only;
952+
using beman::optional26::tests::explicitly_convertible_from_non_const_lvalue_only;
953+
954+
explicitly_convertible_from_non_const_lvalue_only ec;
955+
956+
beman::optional26::optional<explicitly_convertible_from_non_const_lvalue_only&> o3 = ec;
957+
958+
beman::optional26::optional<copyable_from_non_const_lvalue_only> o4(o3);
959+
ASSERT_TRUE(o4);
960+
beman::optional26::optional<copyable_from_non_const_lvalue_only> o5(std::move(o3));
961+
ASSERT_TRUE(o5);
962+
}
963+
964+
TEST(OptionalTest, OptionalFromOptionalConstRef) {
965+
using beman::optional26::tests::copyable_from_const_lvalue_only;
966+
967+
copyable_from_const_lvalue_only cm;
968+
969+
beman::optional26::optional<const copyable_from_const_lvalue_only&> o1 = cm;
970+
ASSERT_TRUE(o1);
971+
972+
{
973+
beman::optional26::optional<copyable_from_const_lvalue_only> o2 = o1;
974+
ASSERT_TRUE(o2);
975+
}
976+
977+
beman::optional26::optional<copyable_from_const_lvalue_only> o2 = std::move(o1);
978+
ASSERT_TRUE(o2);
979+
980+
o2 = o1;
981+
ASSERT_TRUE(o2);
982+
983+
o2 = std::move(o1);
984+
ASSERT_TRUE(o2);
985+
986+
o2.reset();
987+
o2 = o1;
988+
ASSERT_TRUE(o2);
989+
990+
o2.reset();
991+
o2 = std::move(o1);
992+
ASSERT_TRUE(o2);
993+
}
994+
995+
TEST(OptionalTest, OptionalFromOptionalConstRefExplicit) {
996+
using beman::optional26::tests::copyable_from_const_lvalue_only;
997+
using beman::optional26::tests::explicitly_convertible_from_const_lvalue_only;
998+
999+
explicitly_convertible_from_const_lvalue_only ec;
1000+
1001+
beman::optional26::optional<const explicitly_convertible_from_const_lvalue_only&> o3 = ec;
1002+
1003+
beman::optional26::optional<copyable_from_const_lvalue_only> o4(o3);
1004+
ASSERT_TRUE(o4);
1005+
beman::optional26::optional<copyable_from_const_lvalue_only> o5(std::move(o3));
1006+
ASSERT_TRUE(o5);
1007+
}

‎src/beman/optional26/tests/test_types.hpp‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,44 @@ struct immovable {
7171
immovable& operator=(const immovable&) = delete;
7272
};
7373

74+
struct copyable_from_non_const_lvalue_only {
75+
explicit copyable_from_non_const_lvalue_only() = default;
76+
copyable_from_non_const_lvalue_only(copyable_from_non_const_lvalue_only&) = default;
77+
copyable_from_non_const_lvalue_only(const copyable_from_non_const_lvalue_only&) = delete;
78+
copyable_from_non_const_lvalue_only(copyable_from_non_const_lvalue_only&&) = delete;
79+
copyable_from_non_const_lvalue_only(const copyable_from_non_const_lvalue_only&&) = delete;
80+
copyable_from_non_const_lvalue_only& operator=(copyable_from_non_const_lvalue_only&) = default;
81+
copyable_from_non_const_lvalue_only& operator=(const copyable_from_non_const_lvalue_only&) = delete;
82+
copyable_from_non_const_lvalue_only& operator=(copyable_from_non_const_lvalue_only&&) = delete;
83+
copyable_from_non_const_lvalue_only& operator=(const copyable_from_non_const_lvalue_only&&) = delete;
84+
};
85+
86+
struct explicitly_convertible_from_non_const_lvalue_only {
87+
explicit operator copyable_from_non_const_lvalue_only() & { return copyable_from_non_const_lvalue_only{}; }
88+
explicit operator copyable_from_non_const_lvalue_only() const& = delete;
89+
explicit operator copyable_from_non_const_lvalue_only() && = delete;
90+
explicit operator copyable_from_non_const_lvalue_only() const&& = delete;
91+
};
92+
93+
struct copyable_from_const_lvalue_only {
94+
explicit copyable_from_const_lvalue_only() = default;
95+
copyable_from_const_lvalue_only(copyable_from_const_lvalue_only&) = delete;
96+
copyable_from_const_lvalue_only(const copyable_from_const_lvalue_only&) = default;
97+
copyable_from_const_lvalue_only(copyable_from_const_lvalue_only&&) = delete;
98+
copyable_from_const_lvalue_only(const copyable_from_const_lvalue_only&&) = delete;
99+
copyable_from_const_lvalue_only& operator=(copyable_from_const_lvalue_only&) = delete;
100+
copyable_from_const_lvalue_only& operator=(const copyable_from_const_lvalue_only&) = default;
101+
copyable_from_const_lvalue_only& operator=(copyable_from_const_lvalue_only&&) = delete;
102+
copyable_from_const_lvalue_only& operator=(const copyable_from_const_lvalue_only&&) = delete;
103+
};
104+
105+
struct explicitly_convertible_from_const_lvalue_only {
106+
explicit operator copyable_from_const_lvalue_only() & = delete;
107+
explicit operator copyable_from_const_lvalue_only() const& { return copyable_from_const_lvalue_only{}; }
108+
explicit operator copyable_from_const_lvalue_only() && = delete;
109+
explicit operator copyable_from_const_lvalue_only() const&& = delete;
110+
};
111+
74112
} // namespace beman::optional26::tests
75113

76114
#endif // BEMAN_OPTIONAL26_TESTS_TEST_TYPES_HPP

0 commit comments

Comments
 (0)