Author: mpark Date: Wed Jun 7 05:22:43 2017 New Revision: 304891 URL: http://llvm.org/viewvc/llvm-project?rev=304891&view=rev Log: Implement LWG 2904.
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. Reviewers: EricWF, CaseyCarter Reviewed By: EricWF Differential Revision: https://reviews.llvm.org/D33965 Modified: libcxx/trunk/include/variant libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp Modified: libcxx/trunk/include/variant URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=304891&r1=304890&r2=304891&view=diff ============================================================================== --- libcxx/trunk/include/variant (original) +++ libcxx/trunk/include/variant Wed Jun 7 05:22:43 2017 @@ -358,7 +358,6 @@ struct __traits { 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( @@ -877,25 +876,24 @@ public: } 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>>{}); } } @@ -912,8 +910,7 @@ protected: [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)); } @@ -1013,8 +1010,7 @@ public: 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 @@ -1088,7 +1084,6 @@ class _LIBCPP_TEMPLATE_VIS variant __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> { Modified: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp?rev=304891&r1=304890&r2=304891&view=diff ============================================================================== --- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp (original) +++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp Wed Jun 7 05:22:43 2017 @@ -199,12 +199,8 @@ void test_T_assignment_performs_construc 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>; @@ -213,28 +209,6 @@ void test_T_assignment_performs_construc 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 } Modified: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp?rev=304891&r1=304890&r2=304891&view=diff ============================================================================== --- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp (original) +++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp Wed Jun 7 05:22:43 2017 @@ -224,13 +224,7 @@ void test_copy_assignment_sfinae() { } { 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>; @@ -263,12 +257,10 @@ void test_copy_assignment_sfinae() { 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() { @@ -487,41 +479,21 @@ void test_copy_assignment_different_inde 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>; @@ -533,7 +505,6 @@ void test_copy_assignment_different_inde 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>); Modified: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp?rev=304891&r1=304890&r2=304891&view=diff ============================================================================== --- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp (original) +++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp Wed Jun 7 05:22:43 2017 @@ -183,13 +183,7 @@ void test_move_assignment_sfinae() { } { 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>; @@ -232,12 +226,10 @@ void test_move_assignment_sfinae() { 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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
