Commit 5f00d0d5 by Jonathan Wakely Committed by Jonathan Wakely

Fix condition for std::variant to be copy constructible

The standard says the std::variant copy constructor is defined as
deleted unless all alternative types are copy constructible, but we were
making it also depend on move constructible. Fix the condition and
enhance the tests to check the semantics with pathological copy-only
types (i.e. supporting copying but having deleted moves).

The enhanced tests revealed a regression in copy assignment for
non-trivial alternative types, where the assignment would not be
performed because the condition in the _Copy_assign_base visitor is
false: is_same_v<remove_reference_t<T&>, remove_reference_t<const T&>>.

	* include/std/variant (__detail::__variant::_Traits::_S_copy_assign):
	Do not depend on whether all alternative types are move constructible.
	(__detail::__variant::_Copy_assign_base::operator=): Remove cv-quals
	from the operand when deciding whether to perform the assignment.
	* testsuite/20_util/variant/compile.cc (DeletedMoves): Define type
	with deleted move constructor and deleted move assignment operator.
	(default_ctor, copy_ctor, move_ctor, copy_assign, move_assign): Check
	behaviour of variants with DeletedMoves as an alternative.
	* testsuite/20_util/variant/run.cc (DeletedMoves): Define same type.
	(move_ctor, move_assign): Check that moving a variant with a
	DeletedMoves alternative falls back to copying instead of moving.

From-SVN: r270425
parent 990666d0
2019-04-17 Jonathan Wakely <jwakely@redhat.com> 2019-04-17 Jonathan Wakely <jwakely@redhat.com>
* include/std/variant (__detail::__variant::_Traits::_S_copy_assign):
Do not depend on whether all alternative types are move constructible.
(__detail::__variant::_Copy_assign_base::operator=): Remove cv-quals
from the operand when deciding whether to perform the assignment.
* testsuite/20_util/variant/compile.cc (DeletedMoves): Define type
with deleted move constructor and deleted move assignment operator.
(default_ctor, copy_ctor, move_ctor, copy_assign, move_assign): Check
behaviour of variants with DeletedMoves as an alternative.
* testsuite/20_util/variant/run.cc (DeletedMoves): Define same type.
(move_ctor, move_assign): Check that moving a variant with a
DeletedMoves alternative falls back to copying instead of moving.
* testsuite/20_util/variant/compile.cc: Remove empty string literals * testsuite/20_util/variant/compile.cc: Remove empty string literals
from static_assert declarations. from static_assert declarations.
......
...@@ -279,7 +279,7 @@ namespace __variant ...@@ -279,7 +279,7 @@ namespace __variant
static constexpr bool _S_move_ctor = static constexpr bool _S_move_ctor =
(is_move_constructible_v<_Types> && ...); (is_move_constructible_v<_Types> && ...);
static constexpr bool _S_copy_assign = static constexpr bool _S_copy_assign =
_S_copy_ctor && _S_move_ctor _S_copy_ctor
&& (is_copy_assignable_v<_Types> && ...); && (is_copy_assignable_v<_Types> && ...);
static constexpr bool _S_move_assign = static constexpr bool _S_move_assign =
_S_move_ctor _S_move_ctor
...@@ -613,7 +613,7 @@ namespace __variant ...@@ -613,7 +613,7 @@ namespace __variant
__variant::__get<__rhs_index>(*this); __variant::__get<__rhs_index>(*this);
if constexpr (is_same_v< if constexpr (is_same_v<
remove_reference_t<decltype(__this_mem)>, remove_reference_t<decltype(__this_mem)>,
remove_reference_t<decltype(__rhs_mem)>>) __remove_cvref_t<decltype(__rhs_mem)>>)
__this_mem = __rhs_mem; __this_mem = __rhs_mem;
} }
} }
......
...@@ -63,6 +63,15 @@ struct MoveCtorOnly ...@@ -63,6 +63,15 @@ struct MoveCtorOnly
struct MoveCtorAndSwapOnly : MoveCtorOnly { }; struct MoveCtorAndSwapOnly : MoveCtorOnly { };
void swap(MoveCtorAndSwapOnly&, MoveCtorAndSwapOnly&) { } void swap(MoveCtorAndSwapOnly&, MoveCtorAndSwapOnly&) { }
struct DeletedMoves
{
DeletedMoves() = default;
DeletedMoves(const DeletedMoves&) = default;
DeletedMoves(DeletedMoves&&) = delete;
DeletedMoves& operator=(const DeletedMoves&) = default;
DeletedMoves& operator=(DeletedMoves&&) = delete;
};
struct nonliteral struct nonliteral
{ {
nonliteral() { } nonliteral() { }
...@@ -81,6 +90,7 @@ void default_ctor() ...@@ -81,6 +90,7 @@ void default_ctor()
static_assert(is_default_constructible_v<variant<string, string>>); static_assert(is_default_constructible_v<variant<string, string>>);
static_assert(!is_default_constructible_v<variant<AllDeleted, string>>); static_assert(!is_default_constructible_v<variant<AllDeleted, string>>);
static_assert(is_default_constructible_v<variant<string, AllDeleted>>); static_assert(is_default_constructible_v<variant<string, AllDeleted>>);
static_assert(is_default_constructible_v<variant<DeletedMoves>>);
static_assert(noexcept(variant<int>())); static_assert(noexcept(variant<int>()));
static_assert(!noexcept(variant<Empty>())); static_assert(!noexcept(variant<Empty>()));
...@@ -93,6 +103,7 @@ void copy_ctor() ...@@ -93,6 +103,7 @@ void copy_ctor()
static_assert(!is_copy_constructible_v<variant<AllDeleted, string>>); static_assert(!is_copy_constructible_v<variant<AllDeleted, string>>);
static_assert(is_trivially_copy_constructible_v<variant<int>>); static_assert(is_trivially_copy_constructible_v<variant<int>>);
static_assert(!is_trivially_copy_constructible_v<variant<std::string>>); static_assert(!is_trivially_copy_constructible_v<variant<std::string>>);
static_assert(is_trivially_copy_constructible_v<variant<DeletedMoves>>);
{ {
variant<int> a; variant<int> a;
...@@ -116,6 +127,7 @@ void move_ctor() ...@@ -116,6 +127,7 @@ void move_ctor()
{ {
static_assert(is_move_constructible_v<variant<int, string>>); static_assert(is_move_constructible_v<variant<int, string>>);
static_assert(!is_move_constructible_v<variant<AllDeleted, string>>); static_assert(!is_move_constructible_v<variant<AllDeleted, string>>);
static_assert(is_move_constructible_v<variant<int, DeletedMoves>>); // uses copy ctor
static_assert(is_trivially_move_constructible_v<variant<int>>); static_assert(is_trivially_move_constructible_v<variant<int>>);
static_assert(!is_trivially_move_constructible_v<variant<std::string>>); static_assert(!is_trivially_move_constructible_v<variant<std::string>>);
static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>()))); static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>())));
...@@ -157,6 +169,7 @@ void copy_assign() ...@@ -157,6 +169,7 @@ void copy_assign()
static_assert(!is_copy_assignable_v<variant<AllDeleted, string>>); static_assert(!is_copy_assignable_v<variant<AllDeleted, string>>);
static_assert(is_trivially_copy_assignable_v<variant<int>>); static_assert(is_trivially_copy_assignable_v<variant<int>>);
static_assert(!is_trivially_copy_assignable_v<variant<string>>); static_assert(!is_trivially_copy_assignable_v<variant<string>>);
static_assert(is_trivially_copy_assignable_v<variant<DeletedMoves>>);
{ {
variant<Empty> a; variant<Empty> a;
static_assert(!noexcept(a = a)); static_assert(!noexcept(a = a));
...@@ -171,6 +184,7 @@ void move_assign() ...@@ -171,6 +184,7 @@ void move_assign()
{ {
static_assert(is_move_assignable_v<variant<int, string>>); static_assert(is_move_assignable_v<variant<int, string>>);
static_assert(!is_move_assignable_v<variant<AllDeleted, string>>); static_assert(!is_move_assignable_v<variant<AllDeleted, string>>);
static_assert(is_move_assignable_v<variant<int, DeletedMoves>>); // uses copy assignment
static_assert(is_trivially_move_assignable_v<variant<int>>); static_assert(is_trivially_move_assignable_v<variant<int>>);
static_assert(!is_trivially_move_assignable_v<variant<string>>); static_assert(!is_trivially_move_assignable_v<variant<string>>);
{ {
......
...@@ -57,6 +57,15 @@ struct AlwaysThrow ...@@ -57,6 +57,15 @@ struct AlwaysThrow
bool operator>(const AlwaysThrow&) const { VERIFY(false); } bool operator>(const AlwaysThrow&) const { VERIFY(false); }
}; };
struct DeletedMoves
{
DeletedMoves() = default;
DeletedMoves(const DeletedMoves&) = default;
DeletedMoves(DeletedMoves&&) = delete;
DeletedMoves& operator=(const DeletedMoves&) = default;
DeletedMoves& operator=(DeletedMoves&&) = delete;
};
void default_ctor() void default_ctor()
{ {
variant<monostate, string> v; variant<monostate, string> v;
...@@ -80,6 +89,12 @@ void move_ctor() ...@@ -80,6 +89,12 @@ void move_ctor()
VERIFY(holds_alternative<string>(u)); VERIFY(holds_alternative<string>(u));
VERIFY(get<string>(u) == "a"); VERIFY(get<string>(u) == "a");
VERIFY(holds_alternative<string>(v)); VERIFY(holds_alternative<string>(v));
variant<vector<int>, DeletedMoves> d{std::in_place_index<0>, {1, 2, 3, 4}};
// DeletedMoves is not move constructible, so this uses copy ctor:
variant<vector<int>, DeletedMoves> e(std::move(d));
VERIFY(std::get<0>(d).size() == 4);
VERIFY(std::get<0>(e).size() == 4);
} }
void arbitrary_ctor() void arbitrary_ctor()
...@@ -137,6 +152,13 @@ void move_assign() ...@@ -137,6 +152,13 @@ void move_assign()
VERIFY(holds_alternative<string>(u)); VERIFY(holds_alternative<string>(u));
VERIFY(get<string>(u) == "a"); VERIFY(get<string>(u) == "a");
VERIFY(holds_alternative<string>(v)); VERIFY(holds_alternative<string>(v));
variant<vector<int>, DeletedMoves> d{std::in_place_index<0>, {1, 2, 3, 4}};
variant<vector<int>, DeletedMoves> e;
// DeletedMoves is not move assignable, so this uses copy assignment:
e = std::move(d);
VERIFY(std::get<0>(d).size() == 4);
VERIFY(std::get<0>(e).size() == 4);
} }
void arbitrary_assign() void arbitrary_assign()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment