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

git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@304891 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Michael Park
2017-06-07 10:22:43 +00:00
parent 7457967300
commit 3762fe69d2
4 changed files with 8 additions and 76 deletions

View File

@@ -358,7 +358,6 @@ struct __traits {
static constexpr _Trait __copy_assignable_trait = __common_trait( static constexpr _Trait __copy_assignable_trait = __common_trait(
{__copy_constructible_trait, {__copy_constructible_trait,
__move_constructible_trait,
__trait<_Types, is_trivially_copy_assignable, is_copy_assignable>...}); __trait<_Types, is_trivially_copy_assignable, is_copy_assignable>...});
static constexpr _Trait __move_assignable_trait = __common_trait( static constexpr _Trait __move_assignable_trait = __common_trait(
@@ -877,25 +876,24 @@ public:
} }
protected: protected:
template <bool _CopyAssign, size_t _Ip, class _Tp, class _Arg> template <size_t _Ip, class _Tp, class _Arg>
inline _LIBCPP_INLINE_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
void __assign_alt(__alt<_Ip, _Tp>& __a, void __assign_alt(__alt<_Ip, _Tp>& __a, _Arg&& __arg) {
_Arg&& __arg,
bool_constant<_CopyAssign> __tag) {
if (this->index() == _Ip) { if (this->index() == _Ip) {
__a.__value = _VSTD::forward<_Arg>(__arg); __a.__value = _VSTD::forward<_Arg>(__arg);
} else { } else {
struct { struct {
void operator()(true_type) const { 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 { void operator()(false_type) const {
__this->__emplace<_Ip>(_VSTD::forward<_Arg>(__arg)); __this->__emplace<_Ip>(_Tp(_VSTD::forward<_Arg>(__arg)));
} }
__assignment* __this; __assignment* __this;
_Arg&& __arg; _Arg&& __arg;
} __impl{this, _VSTD::forward<_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](auto& __this_alt, auto&& __that_alt) {
this->__assign_alt( this->__assign_alt(
__this_alt, __this_alt,
_VSTD::forward<decltype(__that_alt)>(__that_alt).__value, _VSTD::forward<decltype(__that_alt)>(__that_alt).__value);
is_lvalue_reference<_That>{});
}, },
*this, _VSTD::forward<_That>(__that)); *this, _VSTD::forward<_That>(__that));
} }
@@ -1013,8 +1010,7 @@ public:
inline _LIBCPP_INLINE_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
void __assign(_Arg&& __arg) { void __assign(_Arg&& __arg) {
this->__assign_alt(__access::__base::__get_alt<_Ip>(*this), this->__assign_alt(__access::__base::__get_alt<_Ip>(*this),
_VSTD::forward<_Arg>(__arg), _VSTD::forward<_Arg>(__arg));
false_type{});
} }
inline _LIBCPP_INLINE_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
@@ -1088,7 +1084,6 @@ class _LIBCPP_TEMPLATE_VIS variant
__all<is_move_constructible_v<_Types>...>::value>, __all<is_move_constructible_v<_Types>...>::value>,
private __sfinae_assign_base< private __sfinae_assign_base<
__all<(is_copy_constructible_v<_Types> && __all<(is_copy_constructible_v<_Types> &&
is_move_constructible_v<_Types> &&
is_copy_assignable_v<_Types>)...>::value, is_copy_assignable_v<_Types>)...>::value,
__all<(is_move_constructible_v<_Types> && __all<(is_move_constructible_v<_Types> &&
is_move_assignable_v<_Types>)...>::value> { is_move_assignable_v<_Types>)...>::value> {

View File

@@ -199,12 +199,8 @@ void test_T_assignment_performs_construction() {
assert(false); assert(false);
} catch (...) { /* ... */ } catch (...) { /* ... */
} }
#ifdef _LIBCPP_VERSION // LWG2904
assert(v.valueless_by_exception());
#else // _LIBCPP_VERSION
assert(v.index() == 0); assert(v.index() == 0);
assert(std::get<0>(v) == "hello"); assert(std::get<0>(v) == "hello");
#endif // _LIBCPP_VERSION
} }
{ {
using V = std::variant<ThrowsAssignT, std::string>; using V = std::variant<ThrowsAssignT, std::string>;
@@ -213,28 +209,6 @@ void test_T_assignment_performs_construction() {
assert(v.index() == 0); assert(v.index() == 0);
assert(std::get<0>(v).value == 42); 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 #endif // TEST_HAS_NO_EXCEPTIONS
} }

View File

@@ -224,13 +224,7 @@ void test_copy_assignment_sfinae() {
} }
{ {
using V = std::variant<int, CopyOnly>; 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, ""); static_assert(std::is_copy_assignable<V>::value, "");
#endif // _LIBCPP_VERSION // LWG2904
} }
{ {
using V = std::variant<int, NoCopy>; using V = std::variant<int, NoCopy>;
@@ -263,12 +257,10 @@ void test_copy_assignment_sfinae() {
using V = std::variant<int, TCopyAssignNTMoveAssign>; using V = std::variant<int, TCopyAssignNTMoveAssign>;
static_assert(std::is_trivially_copy_assignable<V>::value, ""); static_assert(std::is_trivially_copy_assignable<V>::value, "");
} }
#ifndef _LIBCPP_VERSION // LWG2904
{ {
using V = std::variant<int, CopyOnly>; using V = std::variant<int, CopyOnly>;
static_assert(std::is_trivially_copy_assignable<V>::value, ""); static_assert(std::is_trivially_copy_assignable<V>::value, "");
} }
#endif // _LIBCPP_VERSION
} }
void test_copy_assignment_empty_empty() { void test_copy_assignment_empty_empty() {
@@ -487,41 +479,21 @@ void test_copy_assignment_different_index() {
assert(false); assert(false);
} catch (...) { /* ... */ } 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, // Test that copy construction is used directly if move construction may throw,
// resulting in a valueless variant if copy throws. // resulting in a valueless variant if copy throws.
assert(v1.valueless_by_exception()); assert(v1.valueless_by_exception());
#endif // _LIBCPP_VERSION // LWG2904
} }
{ {
using V = std::variant<int, MoveThrows, std::string>; using V = std::variant<int, MoveThrows, std::string>;
V v1(std::in_place_type<std::string>, "hello"); V v1(std::in_place_type<std::string>, "hello");
V v2(std::in_place_type<MoveThrows>); V v2(std::in_place_type<MoveThrows>);
assert(MoveThrows::alive == 1); 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. // Test that copy construction is used directly if move construction may throw.
v1 = v2; v1 = v2;
assert(v1.index() == 1); assert(v1.index() == 1);
assert(v2.index() == 1); assert(v2.index() == 1);
assert(MoveThrows::alive == 2); assert(MoveThrows::alive == 2);
#endif // _LIBCPP_VERSION // LWG2904
} }
#ifndef _LIBCPP_VERSION // LWG2904
{ {
// Test that direct copy construction is preferred when it cannot throw. // Test that direct copy construction is preferred when it cannot throw.
using V = std::variant<int, CopyCannotThrow, std::string>; using V = std::variant<int, CopyCannotThrow, std::string>;
@@ -533,7 +505,6 @@ void test_copy_assignment_different_index() {
assert(v2.index() == 1); assert(v2.index() == 1);
assert(CopyCannotThrow::alive == 2); assert(CopyCannotThrow::alive == 2);
} }
#endif // _LIBCPP_VERSION // LWG2904
{ {
using V = std::variant<int, CopyThrows, std::string>; using V = std::variant<int, CopyThrows, std::string>;
V v1(std::in_place_type<CopyThrows>); V v1(std::in_place_type<CopyThrows>);

View File

@@ -183,13 +183,7 @@ void test_move_assignment_sfinae() {
} }
{ {
using V = std::variant<int, CopyOnly>; 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, ""); static_assert(std::is_move_assignable<V>::value, "");
#endif // _LIBCPP_VERSION // LWG2904
} }
{ {
using V = std::variant<int, NoCopy>; using V = std::variant<int, NoCopy>;
@@ -232,12 +226,10 @@ void test_move_assignment_sfinae() {
using V = std::variant<int, TrivialCopyNontrivialMove>; using V = std::variant<int, TrivialCopyNontrivialMove>;
static_assert(!std::is_trivially_move_assignable<V>::value, ""); static_assert(!std::is_trivially_move_assignable<V>::value, "");
} }
#ifndef _LIBCPP_VERSION // LWG2904
{ {
using V = std::variant<int, CopyOnly>; using V = std::variant<int, CopyOnly>;
static_assert(std::is_trivially_move_assignable<V>::value, ""); static_assert(std::is_trivially_move_assignable<V>::value, "");
} }
#endif // _LIBCPP_VERSION // LWG2904
} }
void test_move_assignment_empty_empty() { void test_move_assignment_empty_empty() {