Implement LWG 2904.

[mpark]

Summary

• Removed the move-constructibe requirement from copy-assignable.
• Updated __assign_alt such that we direct initialize if _Tp can be nothrow-constructible from _Arg, or _Tp's move construction can throw. Otherwise, construct a temporary and move it.
• Updated the tests to remove the pre-LWG2904 path.

Depends on D32671.

[CaseyCarter]

Looks good to me, although I am unfamiliar with your variant implementation. I won't approve the revision so that Eric will have to look at it, but consider me signed off.

[EricWF]

LGTM too. please update cxx1z_status.html as well.

[mpark]

Rebased.

include/variant

struct __traits {
 
   static constexpr _Trait __move_constructible_trait =
       __common_trait({__trait<_Types,
                               is_trivially_move_constructible,
                               is_move_constructible>...});
 
   static constexpr _Trait __copy_assignable_trait = __common_trait(
       {__copy_constructible_trait,
-       __move_constructible_trait,
        __trait<_Types, is_trivially_copy_assignable, is_copy_assignable>...});
 
   static constexpr _Trait __move_assignable_trait = __common_trait(
       {__move_constructible_trait,
        __trait<_Types, is_trivially_move_assignable, is_move_assignable>...});
 
   static constexpr _Trait __destructible_trait = __common_trait(
       {__trait<_Types, is_trivially_destructible, is_destructible>...});
auto& __emplace(_Args&&... __args) {
     this->__destroy();
     auto& __res = this->__construct_alt(__access::__base::__get_alt<_Ip>(*this),
                           _VSTD::forward<_Args>(__args)...);
     this->__index = _Ip;
     return __res;
   }
 
 protected:
-  template <bool _CopyAssign, size_t _Ip, class _Tp, class _Arg>
+  template <size_t _Ip, class _Tp, class _Arg>
   inline _LIBCPP_INLINE_VISIBILITY
-  void __assign_alt(__alt<_Ip, _Tp>& __a,
-                    _Arg&& __arg,
-                    bool_constant<_CopyAssign> __tag) {
+  void __assign_alt(__alt<_Ip, _Tp>& __a, _Arg&& __arg) {
     if (this->index() == _Ip) {
       __a.__value = _VSTD::forward<_Arg>(__arg);
     } else {
       struct {
         void operator()(true_type) const {
-          __this->__emplace<_Ip>(_Tp(_VSTD::forward<_Arg>(__arg)));
+          __this->__emplace<_Ip>(_VSTD::forward<_Arg>(__arg));
         }
         void operator()(false_type) const {
-          __this->__emplace<_Ip>(_VSTD::forward<_Arg>(__arg));
+          __this->__emplace<_Ip>(_Tp(_VSTD::forward<_Arg>(__arg)));
         }
         __assignment* __this;
         _Arg&& __arg;
       } __impl{this, _VSTD::forward<_Arg>(__arg)};
-      __impl(__tag);
+      __impl(bool_constant<is_nothrow_constructible_v<_Tp, _Arg> ||
+                           !is_nothrow_move_constructible_v<_Tp>>{});
     }
   }
 
   template <class _That>
   inline _LIBCPP_INLINE_VISIBILITY
   void __generic_assign(_That&& __that) {
     if (this->valueless_by_exception() && __that.valueless_by_exception()) {
       // do nothing.
     } else if (__that.valueless_by_exception()) {
       this->__destroy();
     } else {
       __visitation::__base::__visit_alt_at(
           __that.index(),
           [this](auto& __this_alt, auto&& __that_alt) {
             this->__assign_alt(
                 __this_alt,
-                _VSTD::forward<decltype(__that_alt)>(__that_alt).__value,
-                is_lvalue_reference<_That>{});
+                _VSTD::forward<decltype(__that_alt)>(__that_alt).__value);
           },
           *this, _VSTD::forward<_That>(__that));
     }
   }
 };
 
 template <class _Traits, _Trait = _Traits::__move_assignable_trait>
 class _LIBCPP_TEMPLATE_VIS __move_assignment;

 public:
   using __base_type::__base_type;
   using __base_type::operator=;
 
   template <size_t _Ip, class _Arg>
   inline _LIBCPP_INLINE_VISIBILITY
   void __assign(_Arg&& __arg) {
     this->__assign_alt(__access::__base::__get_alt<_Ip>(*this),
-                       _VSTD::forward<_Arg>(__arg),
-                       false_type{});
+                       _VSTD::forward<_Arg>(__arg));
   }
 
   inline _LIBCPP_INLINE_VISIBILITY
   void __swap(__impl& __that)  {
     if (this->valueless_by_exception() && __that.valueless_by_exception()) {
       // do nothing.
     } else if (this->index() == __that.index()) {
       __visitation::__base::__visit_alt_at(

 
 template <class... _Types>
 class _LIBCPP_TEMPLATE_VIS variant
     : private __sfinae_ctor_base<
           __all<is_copy_constructible_v<_Types>...>::value,
           __all<is_move_constructible_v<_Types>...>::value>,
       private __sfinae_assign_base<
           __all<(is_copy_constructible_v<_Types> &&
-                 is_move_constructible_v<_Types> &&
                  is_copy_assignable_v<_Types>)...>::value,
           __all<(is_move_constructible_v<_Types> &&
                  is_move_assignable_v<_Types>)...>::value> {
   static_assert(0 < sizeof...(_Types),
                 "variant must consist of at least one alternative.");
 
   static_assert(__all<!is_array_v<_Types>...>::value,
                 "variant can not have an array type as an alternative.");

test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp

#ifndef TEST_HAS_NO_EXCEPTIONS
   {
     using V = std::variant<std::string, ThrowsCtorT>;
     V v(std::in_place_type<std::string>, "hello");
     try {
       v = 42;
       assert(false);
     } catch (...) { /* ... */
     }
-#ifdef _LIBCPP_VERSION // LWG2904
-    assert(v.valueless_by_exception());
-#else // _LIBCPP_VERSION
     assert(v.index() == 0);
     assert(std::get<0>(v) == "hello");
-#endif // _LIBCPP_VERSION
   }
   {
     using V = std::variant<ThrowsAssignT, std::string>;
     V v(std::in_place_type<std::string>, "hello");
     v = 42;
     assert(v.index() == 0);
     assert(std::get<0>(v).value == 42);
   }
-#ifdef _LIBCPP_VERSION // LWG2904
-  {
-    // Test that nothrow direct construction is preferred to nothrow move.
-    using V = std::variant<MoveCrashes, std::string>;
-    V v(std::in_place_type<std::string>, "hello");
-    v = 42;
-    assert(v.index() == 0);
-    assert(std::get<0>(v).value == 42);
-  }
-  {
-    // Test that throwing direct construction is preferred to copy-and-move when
-    // move can throw.
-    using V = std::variant<ThrowsCtorTandMove, std::string>;
-    V v(std::in_place_type<std::string>, "hello");
-    try {
-      v = 42;
-      assert(false);
-    } catch(...) { /* ... */
-    }
-    assert(v.valueless_by_exception());
-  }
-#endif // _LIBCPP_VERSION // LWG2904
 #endif // TEST_HAS_NO_EXCEPTIONS
 }
 
 void test_T_assignment_performs_assignment() {
   using namespace RuntimeHelpers;
 #ifndef TEST_HAS_NO_EXCEPTIONS
   {
     using V = std::variant<ThrowsCtorT>;

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

 
 void test_copy_assignment_sfinae() {
   {
     using V = std::variant<int, long>;
     static_assert(std::is_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, CopyOnly>;
-#ifdef _LIBCPP_VERSION // LWG2904
-    // variant only provides copy assignment when both the copy and move
-    // constructors are well formed
-    static_assert(!std::is_copy_assignable<V>::value, "");
-#else // _LIBCPP_VERSION // LWG2904
     static_assert(std::is_copy_assignable<V>::value, "");
-#endif // _LIBCPP_VERSION // LWG2904
   }
   {
     using V = std::variant<int, NoCopy>;
     static_assert(!std::is_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, MoveOnly>;
     static_assert(!std::is_copy_assignable<V>::value, "");
void test_copy_assignment_sfinae() {
   {
     using V = std::variant<int, TCopyAssign>;
     static_assert(std::is_trivially_copy_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, TCopyAssignNTMoveAssign>;
     static_assert(std::is_trivially_copy_assignable<V>::value, "");
   }
-#ifndef _LIBCPP_VERSION // LWG2904
   {
     using V = std::variant<int, CopyOnly>;
     static_assert(std::is_trivially_copy_assignable<V>::value, "");
   }
-#endif // _LIBCPP_VERSION
 }
 
 void test_copy_assignment_empty_empty() {
 #ifndef TEST_HAS_NO_EXCEPTIONS
   using MET = MakeEmptyT;
   {
     using V = std::variant<int, long, MET>;
     V v1(std::in_place_index<0>);
#ifndef TEST_HAS_NO_EXCEPTIONS
     using V = std::variant<int, CopyThrows, std::string>;
     V v1(std::in_place_type<std::string>, "hello");
     V v2(std::in_place_type<CopyThrows>);
     try {
       v1 = v2;
       assert(false);
     } catch (...) { /* ... */
     }
-#ifdef _LIBCPP_VERSION // LWG2904
-    // Test that if copy construction throws then original value is unchanged.
-    assert(v1.index() == 2);
-    assert(std::get<2>(v1) == "hello");
-#else // _LIBCPP_VERSION // LWG2904
     // Test that copy construction is used directly if move construction may throw,
     // resulting in a valueless variant if copy throws.
     assert(v1.valueless_by_exception());
-#endif // _LIBCPP_VERSION // LWG2904
   }
   {
     using V = std::variant<int, MoveThrows, std::string>;
     V v1(std::in_place_type<std::string>, "hello");
     V v2(std::in_place_type<MoveThrows>);
     assert(MoveThrows::alive == 1);
-#ifdef _LIBCPP_VERSION // LWG2904
-    // Test that if move construction throws then the variant is left
-    // valueless by exception.
-    try {
-      v1 = v2;
-      assert(false);
-    } catch (...) { /* ... */
-    }
-    assert(v1.valueless_by_exception());
-    assert(v2.index() == 1);
-    assert(MoveThrows::alive == 1);
-#else // _LIBCPP_VERSION // LWG2904
     // Test that copy construction is used directly if move construction may throw.
     v1 = v2;
     assert(v1.index() == 1);
     assert(v2.index() == 1);
     assert(MoveThrows::alive == 2);
-#endif // _LIBCPP_VERSION // LWG2904
   }
-#ifndef _LIBCPP_VERSION // LWG2904
   {
     // Test that direct copy construction is preferred when it cannot throw.
     using V = std::variant<int, CopyCannotThrow, std::string>;
     V v1(std::in_place_type<std::string>, "hello");
     V v2(std::in_place_type<CopyCannotThrow>);
     assert(CopyCannotThrow::alive == 1);
     v1 = v2;
     assert(v1.index() == 1);
     assert(v2.index() == 1);
     assert(CopyCannotThrow::alive == 2);
   }
-#endif // _LIBCPP_VERSION // LWG2904
   {
     using V = std::variant<int, CopyThrows, std::string>;
     V v1(std::in_place_type<CopyThrows>);
     V v2(std::in_place_type<std::string>, "hello");
     V &vref = (v1 = v2);
     assert(&vref == &v1);
     assert(v1.index() == 2);
     assert(std::get<2>(v1) == "hello");

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp

 
 void test_move_assignment_sfinae() {
   {
     using V = std::variant<int, long>;
     static_assert(std::is_move_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, CopyOnly>;
-#ifdef _LIBCPP_VERSION // LWG2904
-    // variant only provides move assignment when both the move constructor
-    // and move assignment operator are well formed.
-    static_assert(!std::is_move_assignable<V>::value, "");
-#else // _LIBCPP_VERSION // LWG2904
     static_assert(std::is_move_assignable<V>::value, "");
-#endif // _LIBCPP_VERSION // LWG2904
   }
   {
     using V = std::variant<int, NoCopy>;
     static_assert(!std::is_move_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, MoveOnly>;
     static_assert(std::is_move_assignable<V>::value, "");
void test_move_assignment_sfinae() {
   {
     using V = std::variant<int, TMoveAssignNTCopyAssign>;
     static_assert(std::is_trivially_move_assignable<V>::value, "");
   }
   {
     using V = std::variant<int, TrivialCopyNontrivialMove>;
     static_assert(!std::is_trivially_move_assignable<V>::value, "");
   }
-#ifndef _LIBCPP_VERSION // LWG2904
   {
     using V = std::variant<int, CopyOnly>;
     static_assert(std::is_trivially_move_assignable<V>::value, "");
   }
-#endif // _LIBCPP_VERSION // LWG2904
 }
 
 void test_move_assignment_empty_empty() {
 #ifndef TEST_HAS_NO_EXCEPTIONS
   using MET = MakeEmptyT;
   {
     using V = std::variant<int, long, MET>;
     V v1(std::in_place_index<0>);