[libc++] Implement LWG3938 (Cannot use std::expected monadic ops with move-only error_type)

Implement LWG3938 (Cannot use std::expected monadic ops with move-only error_type)
https://wg21.link/LWG3938

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D154116

Author: yronglin

libcxx/docs/Status/Cxx2cIssues.csv

@@ -14,7 +14,7 @@
 "`3925 <https://wg21.link/LWG3925>`__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|Complete|","17.0","|format|"
 "`3927 <https://wg21.link/LWG3927>`__","Unclear preconditions for ``operator[]`` for sequence containers","Varna June 2023","|Nothing To Do|","",""
 "`3935 <https://wg21.link/LWG3935>`__","``template<class X> constexpr complex& operator=(const complex<X>&)`` has no specification","Varna June 2023","|Complete|","3.4",""
-"`3938 <https://wg21.link/LWG3938>`__","Cannot use ``std::expected`` monadic ops with move-only ``error_type``","Varna June 2023","","",""
+"`3938 <https://wg21.link/LWG3938>`__","Cannot use ``std::expected`` monadic ops with move-only ``error_type``","Varna June 2023","|Complete|","18.0",""
 "`3940 <https://wg21.link/LWG3940>`__","``std::expected<void, E>::value()`` also needs ``E`` to be copy constructible","Varna June 2023","","",""
 "","","","","",""
 "`3343 <https://wg21.link/LWG3343>`__","Ordering of calls to ``unlock()`` and ``notify_all()`` in Effects element of ``notify_all_at_thread_exit()`` should be reversed","Not Yet Adopted","|Complete|","16.0",""

libcxx/include/__expected/expected.h

@@ -651,11 +651,11 @@ class expected {
     requires is_constructible_v<_Err, _Err&>
   _LIBCPP_HIDE_FROM_ABI constexpr auto and_then(_Func&& __f) & {
     using _Up = remove_cvref_t<invoke_result_t<_Func, _Tp&>>;
-    static_assert(__is_std_expected<_Up>::value, "The result of f(value()) must be a specialization of std::expected");
+    static_assert(__is_std_expected<_Up>::value, "The result of f(**this) must be a specialization of std::expected");
     static_assert(is_same_v<typename _Up::error_type, _Err>,
-                  "The result of f(value()) must have the same error_type as this expected");
+                  "The result of f(**this) must have the same error_type as this expected");
     if (has_value()) {
-      return std::invoke(std::forward<_Func>(__f), value());
+      return std::invoke(std::forward<_Func>(__f), __union_.__val_);
     }
     return _Up(unexpect, error());
   }
@@ -664,11 +664,11 @@ class expected {
     requires is_constructible_v<_Err, const _Err&>
   _LIBCPP_HIDE_FROM_ABI constexpr auto and_then(_Func&& __f) const& {
     using _Up = remove_cvref_t<invoke_result_t<_Func, const _Tp&>>;
-    static_assert(__is_std_expected<_Up>::value, "The result of f(value()) must be a specialization of std::expected");
+    static_assert(__is_std_expected<_Up>::value, "The result of f(**this) must be a specialization of std::expected");
     static_assert(is_same_v<typename _Up::error_type, _Err>,
-                  "The result of f(value()) must have the same error_type as this expected");
+                  "The result of f(**this) must have the same error_type as this expected");
     if (has_value()) {
-      return std::invoke(std::forward<_Func>(__f), value());
+      return std::invoke(std::forward<_Func>(__f), __union_.__val_);
     }
     return _Up(unexpect, error());
   }
@@ -678,11 +678,11 @@ class expected {
   _LIBCPP_HIDE_FROM_ABI constexpr auto and_then(_Func&& __f) && {
     using _Up = remove_cvref_t<invoke_result_t<_Func, _Tp&&>>;
     static_assert(
-        __is_std_expected<_Up>::value, "The result of f(std::move(value())) must be a specialization of std::expected");
+        __is_std_expected<_Up>::value, "The result of f(std::move(**this)) must be a specialization of std::expected");
     static_assert(is_same_v<typename _Up::error_type, _Err>,
-                  "The result of f(std::move(value())) must have the same error_type as this expected");
+                  "The result of f(std::move(**this)) must have the same error_type as this expected");
     if (has_value()) {
-      return std::invoke(std::forward<_Func>(__f), std::move(value()));
+      return std::invoke(std::forward<_Func>(__f), std::move(__union_.__val_));
     }
     return _Up(unexpect, std::move(error()));
   }
@@ -692,11 +692,11 @@ class expected {
   _LIBCPP_HIDE_FROM_ABI constexpr auto and_then(_Func&& __f) const&& {
     using _Up = remove_cvref_t<invoke_result_t<_Func, const _Tp&&>>;
     static_assert(
-        __is_std_expected<_Up>::value, "The result of f(std::move(value())) must be a specialization of std::expected");
+        __is_std_expected<_Up>::value, "The result of f(std::move(**this)) must be a specialization of std::expected");
     static_assert(is_same_v<typename _Up::error_type, _Err>,
-                  "The result of f(std::move(value())) must have the same error_type as this expected");
+                  "The result of f(std::move(**this)) must have the same error_type as this expected");
     if (has_value()) {
-      return std::invoke(std::forward<_Func>(__f), std::move(value()));
+      return std::invoke(std::forward<_Func>(__f), std::move(__union_.__val_));
     }
     return _Up(unexpect, std::move(error()));
   }
@@ -709,7 +709,7 @@ class expected {
     static_assert(is_same_v<typename _Gp::value_type, _Tp>,
                   "The result of f(error()) must have the same value_type as this expected");
     if (has_value()) {
-      return _Gp(in_place, value());
+      return _Gp(in_place, __union_.__val_);
     }
     return std::invoke(std::forward<_Func>(__f), error());
   }
@@ -722,7 +722,7 @@ class expected {
     static_assert(is_same_v<typename _Gp::value_type, _Tp>,
                   "The result of f(error()) must have the same value_type as this expected");
     if (has_value()) {
-      return _Gp(in_place, value());
+      return _Gp(in_place, __union_.__val_);
     }
     return std::invoke(std::forward<_Func>(__f), error());
   }
@@ -736,7 +736,7 @@ class expected {
     static_assert(is_same_v<typename _Gp::value_type, _Tp>,
                   "The result of f(std::move(error())) must have the same value_type as this expected");
     if (has_value()) {
-      return _Gp(in_place, std::move(value()));
+      return _Gp(in_place, std::move(__union_.__val_));
     }
     return std::invoke(std::forward<_Func>(__f), std::move(error()));
   }
@@ -750,7 +750,7 @@ class expected {
     static_assert(is_same_v<typename _Gp::value_type, _Tp>,
                   "The result of f(std::move(error())) must have the same value_type as this expected");
     if (has_value()) {
-      return _Gp(in_place, std::move(value()));
+      return _Gp(in_place, std::move(__union_.__val_));
     }
     return std::invoke(std::forward<_Func>(__f), std::move(error()));
   }
@@ -763,9 +763,9 @@ class expected {
       return expected<_Up, _Err>(unexpect, error());
     }
     if constexpr (!is_void_v<_Up>) {
-      return expected<_Up, _Err>(__expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), value());
+      return expected<_Up, _Err>(__expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), __union_.__val_);
     } else {
-      std::invoke(std::forward<_Func>(__f), value());
+      std::invoke(std::forward<_Func>(__f), __union_.__val_);
       return expected<_Up, _Err>();
     }
   }
@@ -778,9 +778,9 @@ class expected {
       return expected<_Up, _Err>(unexpect, error());
     }
     if constexpr (!is_void_v<_Up>) {
-      return expected<_Up, _Err>(__expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), value());
+      return expected<_Up, _Err>(__expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), __union_.__val_);
     } else {
-      std::invoke(std::forward<_Func>(__f), value());
+      std::invoke(std::forward<_Func>(__f), __union_.__val_);
       return expected<_Up, _Err>();
     }
   }
@@ -794,9 +794,9 @@ class expected {
     }
     if constexpr (!is_void_v<_Up>) {
       return expected<_Up, _Err>(
-          __expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), std::move(value()));
+          __expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), std::move(__union_.__val_));
     } else {
-      std::invoke(std::forward<_Func>(__f), std::move(value()));
+      std::invoke(std::forward<_Func>(__f), std::move(__union_.__val_));
       return expected<_Up, _Err>();
     }
   }
@@ -810,9 +810,9 @@ class expected {
     }
     if constexpr (!is_void_v<_Up>) {
       return expected<_Up, _Err>(
-          __expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), std::move(value()));
+          __expected_construct_in_place_from_invoke_tag{}, std::forward<_Func>(__f), std::move(__union_.__val_));
     } else {
-      std::invoke(std::forward<_Func>(__f), std::move(value()));
+      std::invoke(std::forward<_Func>(__f), std::move(__union_.__val_));
       return expected<_Up, _Err>();
     }
   }
@@ -824,7 +824,7 @@ class expected {
     static_assert(__valid_std_unexpected<_Gp>::value,
                   "The result of f(error()) must be a valid template argument for unexpected");
     if (has_value()) {
-      return expected<_Tp, _Gp>(in_place, value());
+      return expected<_Tp, _Gp>(in_place, __union_.__val_);
     }
     return expected<_Tp, _Gp>(__expected_construct_unexpected_from_invoke_tag{}, std::forward<_Func>(__f), error());
   }
@@ -836,7 +836,7 @@ class expected {
     static_assert(__valid_std_unexpected<_Gp>::value,
                   "The result of f(error()) must be a valid template argument for unexpected");
     if (has_value()) {
-      return expected<_Tp, _Gp>(in_place, value());
+      return expected<_Tp, _Gp>(in_place, __union_.__val_);
     }
     return expected<_Tp, _Gp>(__expected_construct_unexpected_from_invoke_tag{}, std::forward<_Func>(__f), error());
   }
@@ -848,7 +848,7 @@ class expected {
     static_assert(__valid_std_unexpected<_Gp>::value,
                   "The result of f(std::move(error())) must be a valid template argument for unexpected");
     if (has_value()) {
-      return expected<_Tp, _Gp>(in_place, std::move(value()));
+      return expected<_Tp, _Gp>(in_place, std::move(__union_.__val_));
     }
     return expected<_Tp, _Gp>(
         __expected_construct_unexpected_from_invoke_tag{}, std::forward<_Func>(__f), std::move(error()));
@@ -861,7 +861,7 @@ class expected {
     static_assert(__valid_std_unexpected<_Gp>::value,
                   "The result of f(std::move(error())) must be a valid template argument for unexpected");
     if (has_value()) {
-      return expected<_Tp, _Gp>(in_place, std::move(value()));
+      return expected<_Tp, _Gp>(in_place, std::move(__union_.__val_));
     }
     return expected<_Tp, _Gp>(
         __expected_construct_unexpected_from_invoke_tag{}, std::forward<_Func>(__f), std::move(error()));

libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp

@@ -11,22 +11,22 @@
 // Test the mandates
 // template<class F> constexpr auto and_then(F&& f) &;
 // Mandates:
-//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(value())>>
+//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(**this)>>
 //  U is a specialization of std::expected and std::is_same_v<U:error_type, E> is true
 
 // template<class F> constexpr auto and_then(F&& f) const &;
 // Mandates:
-//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(value())>>
+//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(**this)>>
 //  U is a specialization of std::expected and std::is_same_v<U:error_type, E> is true
 
 // template<class F> constexpr auto and_then(F&& f) &&;
 // Mandates:
-//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(value())>>
+//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(**this)>>
 //  U is a specialization of std::expected and std::is_same_v<U:error_type, E> is true
 
 // template<class F> constexpr auto and_then(F&& f) const &&;
 // Mandates:
-//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(value())>>
+//  Let U be std::remove_cvref_t<std::invoke_result<F, decltype(**this)>>
 //  U is a specialization of std::expected and std::is_same_v<U:error_type, E> is true
 
 #include <expected>
@@ -52,7 +52,7 @@ void test() {
     {
       std::expected<int, int> f1(1);
       f1.and_then(lval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(value()) must be a specialization of std::expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(**this) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
     }
@@ -61,7 +61,7 @@ void test() {
     {
       std::expected<int, int> f1(1);
       f1.and_then(lval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(value()) must have the same error_type as this expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(**this) must have the same error_type as this expected}}
     }
   }
 
@@ -71,7 +71,7 @@ void test() {
     {
       const std::expected<int, int> f1(1);
       f1.and_then(clval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(value()) must be a specialization of std::expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(**this) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
     }
@@ -80,7 +80,7 @@ void test() {
     {
       const std::expected<int, int> f1(1);
       f1.and_then(clval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(value()) must have the same error_type as this expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(**this) must have the same error_type as this expected}}
 
     }
   }
@@ -91,7 +91,7 @@ void test() {
     {
       std::expected<int, int> f1(1);
       std::move(f1).and_then(rval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &&)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(value())) must be a specialization of std::expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(**this)) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
     }
@@ -100,7 +100,7 @@ void test() {
     {
       std::expected<int, int> f1(1);
       std::move(f1).and_then(rval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &&)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(value())) must have the same error_type as this expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(**this)) must have the same error_type as this expected}}
     }
   }
 
@@ -110,7 +110,7 @@ void test() {
     {
       const std::expected<int, int> f1(1);
       std::move(f1).and_then(crval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &&)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(value())) must be a specialization of std::expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(**this)) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
     }
@@ -119,7 +119,7 @@ void test() {
     {
       const std::expected<int, int> f1(1);
       std::move(f1).and_then(crval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &&)>' requested here}}
-      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(value())) must have the same error_type as this expected}}
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}The result of f(std::move(**this)) must have the same error_type as this expected}}
     }
   }
 }

libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp

@@ -22,6 +22,8 @@
 #include <type_traits>
 #include <utility>
 
+#include "../../types.h"
+
 struct LVal {
   constexpr std::expected<int, int> operator()(int&) { return 1; }
   std::expected<int, int> operator()(const int&)  = delete;
@@ -153,6 +155,7 @@ concept has_and_then = requires(E&& e, F&& f) {
   {std::forward<E>(e).and_then(std::forward<F>(f))};
 };
 
+// clang-format off
 static_assert( has_and_then<std::expected<int, int>&, std::expected<int, int>(int&)>);
 static_assert(!has_and_then<std::expected<int, NonCopyable>&, std::expected<int, NonCopyable>(int&)>);
 static_assert( has_and_then<const std::expected<int, int>&, std::expected<int, int>(const int&)>);
@@ -166,7 +169,11 @@ static_assert(!has_and_then<const std::expected<int, NonMovable>&&, std::expecte
 static_assert(!has_and_then<const std::expected<int, std::unique_ptr<int>>&, int()>);
 static_assert(!has_and_then<const std::expected<int, std::unique_ptr<int>>&&, int()>);
 
-// clang-format off
+// [LWG 3983] https://cplusplus.github.io/LWG/issue3938, check std::expected monadic ops well-formed with move-only error_type.
+// There are no effects for `&` and `const &` overload, because the constraints requires is_constructible_v<E, decltype(error())> is true.
+static_assert(has_and_then<std::expected<int, MoveOnlyErrorType>&&, std::expected<int, MoveOnlyErrorType>(int)>);
+static_assert(has_and_then<const std::expected<int, MoveOnlyErrorType>&&, std::expected<int, MoveOnlyErrorType>(const int)>);
+
 constexpr void test_val_types() {
   // Test & overload
   {
@@ -260,9 +267,26 @@ constexpr void test_sfinae() {
   std::move(e).and_then(l);
 }
 
+constexpr void test_move_only_error_type() {
+  // Test &&
+  {
+    std::expected<int, MoveOnlyErrorType> e;
+    auto l = [](int) { return std::expected<int, MoveOnlyErrorType>{}; };
+    std::move(e).and_then(l);
+  }
+
+  // Test const&&
+  {
+    const std::expected<int, MoveOnlyErrorType> e;
+    auto l = [](const int) { return std::expected<int, MoveOnlyErrorType>{}; };
+    std::move(e).and_then(l);
+  }
+}
+
 constexpr bool test() {
   test_sfinae();
   test_val_types();
+  test_move_only_error_type();
 
   std::expected<int, int> e(std::unexpected<int>(1));
   const auto& ce = e;