[libc++] Implement Resolution of LWG 3886 (#155356)

smallp-o-p · web-flow · commit fb72f0ca7d6c · 2025-09-24T07:26:40.000+08:00
Resolves #118336 - Implement the resolution of [LWG3886](https://cplusplus.github.io/LWG/issue3886) for `optional` and `expected`
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -78,7 +78,7 @@
 "","","","","",""
 "`LWG3216 <https://wg21.link/LWG3216>`__","Rebinding the allocator before calling ``construct``/``destroy`` in ``allocate_shared``","2024-11 (Wrocław)","","","`#118332 <https://github.com/llvm/llvm-project/issues/118332>`__",""
 "`LWG3436 <https://wg21.link/LWG3436>`__","``std::construct_at`` should support arrays","2024-11 (Wrocław)","","","`#118335 <https://github.com/llvm/llvm-project/issues/118335>`__",""
-"`LWG3886 <https://wg21.link/LWG3886>`__","Monad mo' problems","2024-11 (Wrocław)","","","`#118336 <https://github.com/llvm/llvm-project/issues/118336>`__",""
+"`LWG3886 <https://wg21.link/LWG3886>`__","Monad mo' problems","2024-11 (Wrocław)","|Complete|","22","`#118336 <https://github.com/llvm/llvm-project/issues/118336>`__",""
 "`LWG3899 <https://wg21.link/LWG3899>`__","``co_yield``\ing elements of an lvalue generator is unnecessarily inefficient","2024-11 (Wrocław)","","","`#118337 <https://github.com/llvm/llvm-project/issues/118337>`__",""
 "`LWG3900 <https://wg21.link/LWG3900>`__","The ``allocator_arg_t`` overloads of ``generator::promise_type::operator new`` should not be constrained","2024-11 (Wrocław)","","","`#118338 <https://github.com/llvm/llvm-project/issues/118338>`__",""
 "`LWG3918 <https://wg21.link/LWG3918>`__","``std::uninitialized_move/_n`` and guaranteed copy elision","2024-11 (Wrocław)","","","`#118339 <https://github.com/llvm/llvm-project/issues/118339>`__",""
diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
@@ -555,7 +555,7 @@ class expected : private __expected_base<_Tp, _Err> {
           is_nothrow_constructible_v<_Tp, _Up> && is_nothrow_constructible_v<_Err, _OtherErr>) // strengthened
       : __base(__other.__has_val(), std::move(__other.__union())) {}
 
-  template <class _Up = _Tp>
+  template <class _Up = remove_cv_t<_Tp>>
     requires(!is_same_v<remove_cvref_t<_Up>, in_place_t> && !is_same_v<expected, remove_cvref_t<_Up>> &&
              !is_same_v<remove_cvref_t<_Up>, unexpect_t> && is_constructible_v<_Tp, _Up> &&
              !__is_std_unexpected<remove_cvref_t<_Up>>::value &&
@@ -669,7 +669,7 @@ class expected : private __expected_base<_Tp, _Err> {
     return *this;
   }
 
-  template <class _Up = _Tp>
+  template <class _Up = remove_cv_t<_Tp>>
   _LIBCPP_HIDE_FROM_ABI constexpr expected& operator=(_Up&& __v)
     requires(!is_same_v<expected, remove_cvref_t<_Up>> && !__is_std_unexpected<remove_cvref_t<_Up>>::value &&
              is_constructible_v<_Tp, _Up> && is_assignable_v<_Tp&, _Up> &&
@@ -887,14 +887,14 @@ class expected : private __expected_base<_Tp, _Err> {
     return std::move(this->__unex());
   }
 
-  template <class _Up>
+  template <class _Up = remove_cv_t<_Tp>>
   _LIBCPP_HIDE_FROM_ABI constexpr _Tp value_or(_Up&& __v) const& {
     static_assert(is_copy_constructible_v<_Tp>, "value_type has to be copy constructible");
     static_assert(is_convertible_v<_Up, _Tp>, "argument has to be convertible to value_type");
     return this->__has_val() ? this->__val() : static_cast<_Tp>(std::forward<_Up>(__v));
   }
 
-  template <class _Up>
+  template <class _Up = remove_cv_t<_Tp>>
   _LIBCPP_HIDE_FROM_ABI constexpr _Tp value_or(_Up&& __v) && {
     static_assert(is_move_constructible_v<_Tp>, "value_type has to be move constructible");
     static_assert(is_convertible_v<_Up, _Tp>, "argument has to be convertible to value_type");
diff --git a/libcxx/include/optional b/libcxx/include/optional
@@ -119,7 +119,7 @@ namespace std {
       constexpr explicit optional(in_place_t, Args &&...);
     template<class U, class... Args>
       constexpr explicit optional(in_place_t, initializer_list<U>, Args &&...);
-    template<class U = T>
+    template<class U = remove_cv_t<T>>
       constexpr explicit(see-below) optional(U &&);
     template<class U>
       explicit(see-below) optional(const optional<U> &);                          // constexpr in C++20
@@ -133,7 +133,7 @@ namespace std {
     optional &operator=(nullopt_t) noexcept;                                      // constexpr in C++20
     constexpr optional &operator=(const optional &);
     constexpr optional &operator=(optional &&) noexcept(see below);
-    template<class U = T> optional &operator=(U &&);                              // constexpr in C++20
+    template<class U = remove_cv_t<T>> optional &operator=(U &&);                              // constexpr in C++20
     template<class U> optional &operator=(const optional<U> &);                   // constexpr in C++20
     template<class U> optional &operator=(optional<U> &&);                        // constexpr in C++20
     template<class... Args> T& emplace(Args &&...);                               // constexpr in C++20
@@ -161,8 +161,8 @@ namespace std {
     constexpr T &value() &;
     constexpr T &&value() &&;
     constexpr const T &&value() const &&;
-    template<class U> constexpr T value_or(U &&) const &;
-    template<class U> constexpr T value_or(U &&) &&;
+    template<class U = remove_cv_t<T>> constexpr T value_or(U &&) const &;
+    template<class U = remove_cv_t<T>> constexpr T value_or(U &&) &&;
 
     // [optional.monadic], monadic operations
     template<class F> constexpr auto and_then(F&& f) &;         // since C++23
@@ -730,7 +730,8 @@ public:
             enable_if_t<_CheckOptionalArgsCtor<_Up>::template __enable_implicit<_Up>(), int> = 0>
   _LIBCPP_HIDE_FROM_ABI constexpr optional(_Up&& __v) : __base(in_place, std::forward<_Up>(__v)) {}
 
-  template <class _Up, enable_if_t<_CheckOptionalArgsCtor<_Up>::template __enable_explicit<_Up>(), int> = 0>
+  template <class _Up                                                                        = remove_cv_t<_Tp>,
+            enable_if_t<_CheckOptionalArgsCtor<_Up>::template __enable_explicit<_Up>(), int> = 0>
   _LIBCPP_HIDE_FROM_ABI constexpr explicit optional(_Up&& __v) : __base(in_place, std::forward<_Up>(__v)) {}
 
   // LWG2756: conditionally explicit conversion from const optional<_Up>&
@@ -771,7 +772,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI constexpr optional& operator=(optional&&)      = default;
 
   // LWG2756
-  template <class _Up        = value_type,
+  template <class _Up        = remove_cv_t<value_type>,
             enable_if_t<_And<_IsNotSame<__remove_cvref_t<_Up>, optional>,
                              _Or<_IsNotSame<__remove_cvref_t<_Up>, value_type>, _Not<is_scalar<value_type>>>,
                              is_constructible<value_type, _Up>,
@@ -919,14 +920,14 @@ public:
     return std::move(this->__get());
   }
 
-  template <class _Up>
+  template <class _Up = remove_cv_t<_Tp>>
   _LIBCPP_HIDE_FROM_ABI constexpr value_type value_or(_Up&& __v) const& {
     static_assert(is_copy_constructible_v<value_type>, "optional<T>::value_or: T must be copy constructible");
     static_assert(is_convertible_v<_Up, value_type>, "optional<T>::value_or: U must be convertible to T");
     return this->has_value() ? this->__get() : static_cast<value_type>(std::forward<_Up>(__v));
   }
 
-  template <class _Up>
+  template <class _Up = remove_cv_t<_Tp>>
   _LIBCPP_HIDE_FROM_ABI constexpr value_type value_or(_Up&& __v) && {
     static_assert(is_move_constructible_v<value_type>, "optional<T>::value_or: T must be move constructible");
     static_assert(is_convertible_v<_Up, value_type>, "optional<T>::value_or: U must be convertible to T");
diff --git a/libcxx/test/std/utilities/expected/expected.expected/assign/assign.U.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/assign/assign.U.pass.cpp
@@ -325,6 +325,44 @@ constexpr bool test() {
     }
   }
 
+  // Check move constructor selection
+  {
+    struct MoveOnlyMulti {
+      bool used_move1 = false;
+      bool used_move2 = false;
+
+      constexpr MoveOnlyMulti()                                = default;
+      constexpr MoveOnlyMulti(const MoveOnlyMulti&)            = delete;
+      constexpr MoveOnlyMulti& operator=(const MoveOnlyMulti&) = delete;
+      constexpr MoveOnlyMulti& operator=(MoveOnlyMulti&&) {
+        used_move1 = true;
+        return *this;
+      }
+      constexpr MoveOnlyMulti& operator=(const MoveOnlyMulti&&) {
+        used_move2 = true;
+        return *this;
+      };
+      constexpr MoveOnlyMulti(MoveOnlyMulti&&) : used_move1(true) {}
+      constexpr MoveOnlyMulti(const MoveOnlyMulti&&) : used_move2(true) {}
+    };
+
+    {
+      MoveOnlyMulti t{};
+      std::expected<MoveOnlyMulti, int> e1(std::unexpect);
+      static_assert(std::is_same_v<decltype(std::move(t)), MoveOnlyMulti&&>);
+      e1 = {std::move(t)};
+      assert(e1.value().used_move1);
+    }
+    {
+      const MoveOnlyMulti t{};
+      std::expected<MoveOnlyMulti, int> e1(std::unexpect);
+      static_assert(std::is_same_v<decltype(std::move(t)), const MoveOnlyMulti&&>);
+      // _Up = remove_cv_t<const MoveOnlyMulti&&> --> should use MoveOnlyMulti(MoveOnlyMulti&&)
+      e1 = {std::move(t)};
+      assert(e1.value().used_move1);
+    }
+  }
+
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp
@@ -80,6 +80,17 @@ struct CopyOnly {
   friend constexpr bool operator==(const CopyOnly& mi, int ii) { return mi.i == ii; }
 };
 
+struct MoveOnly2 {
+  int j;
+  bool used_move1 = false;
+  bool used_move2 = false;
+
+  constexpr explicit MoveOnly2(int jj) : j(jj) {}
+  constexpr MoveOnly2(const MoveOnly2&) = delete;
+  constexpr MoveOnly2(MoveOnly2&& m) : j(m.j), used_move1(true) {}
+  constexpr MoveOnly2(const MoveOnly2&& m) : j(m.j), used_move2(true) {}
+};
+
 struct BaseError {};
 struct DerivedError : BaseError {};
 
@@ -164,6 +175,22 @@ constexpr bool test() {
     assert(e2.has_value());
     assert(!e2.value()); // yes, e2 holds "false" since LWG3836
   }
+
+  // Check move constructor selection
+  {
+    MoveOnly2 t{1};
+    std::expected<MoveOnly2, BaseError> e1(std::move(t));
+    assert(e1.has_value());
+    assert(e1.value().used_move1 == true);
+    assert(e1.value().j == 1);
+  }
+  {
+    const MoveOnly2 t2{2};
+    std::expected<MoveOnly2, BaseError> e1(std::move(t2));
+    assert(e1.has_value());
+    assert(e1.value().used_move2 == true);
+    assert(e1.value().j == 2);
+  }
   return true;
 }
 
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/U.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/U.pass.cpp
@@ -59,7 +59,8 @@ constexpr bool explicit_conversion(Input&& in, const Expect& v)
     static_assert(!std::is_constructible<O, void*>::value, "");
     static_assert(!std::is_constructible<O, Input, int>::value, "");
     optional<To> opt(std::forward<Input>(in));
-    return opt && *opt == static_cast<To>(v);
+    optional<To> opt2{std::forward<Input>(in)};
+    return opt && *opt == static_cast<To>(v) && (opt2 && *opt2 == static_cast<To>(v));
 }
 
 void test_implicit()
@@ -83,6 +84,11 @@ void test_implicit()
         using T = TestTypes::TestType;
         assert(implicit_conversion<T>(3, T(3)));
     }
+    {
+      using T = TestTypes::TestType;
+      optional<T> opt({3});
+      assert(opt && *opt == static_cast<T>(3));
+    }
   {
     using O = optional<ImplicitAny>;
     static_assert(!test_convertible<O, std::in_place_t>(), "");
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -40,6 +40,12 @@ struct X
         {return x.i_ == y.i_;}
 };
 
+struct Z {
+  int i_, j_;
+  constexpr Z(int i, int j) : i_(i), j_(j) {}
+  friend constexpr bool operator==(const Z& z1, const Z& z2) { return z1.i_ == z2.i_ && z1.j_ == z2.j_; }
+};
+
 constexpr int test()
 {
     {
@@ -64,6 +70,16 @@ constexpr int test()
         assert(std::move(opt).value_or(Y(3)) == 4);
         assert(!opt);
     }
+    {
+      optional<X> opt;
+      assert(std::move(opt).value_or({Y(3)}) == 4);
+      assert(!opt);
+    }
+    {
+      optional<Z> opt;
+      assert((std::move(opt).value_or({2, 3}) == Z{2, 3}));
+      assert(!opt);
+    }
     return 0;
 }
 
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/value_or_const.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/value_or_const.pass.cpp
@@ -75,6 +75,10 @@ int main(int, char**)
         const optional<X> opt;
         assert(opt.value_or(Y(3)) == 4);
     }
+    {
+      const optional<X> opt;
+      assert(opt.value_or({Y(3)}) == 4);
+    }
 
   return 0;
 }

Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,10 @@ int main(int, char**)`
`75`	`75`	`const optional<X> opt;`
`76`	`76`	`assert(opt.value_or(Y(3)) == 4);`
`77`	`77`	`}`
	`78`	`+ {`
	`79`	`+ const optional<X> opt;`
	`80`	`+ assert(opt.value_or({Y(3)}) == 4);`
	`81`	`+ }`
`78`	`82`
`79`	`83`	`return 0;`
`80`	`84`	`}`