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

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 701e349693c8df06044ab53525b3a2029b38534e..0d5e490befe1a31bfc5dbc8a25ac6b2503be19df 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,17 @@
 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
        from static_assert declarations.
 

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 22b0c3d5c2247fb4d68ed721a1521840e46b168c..e153363bbf39f046499733ed5a05e333c0674f61 100644 (file)
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -279,7 +279,7 @@ namespace __variant
       static constexpr bool _S_move_ctor =
          (is_move_constructible_v<_Types> && ...);
       static constexpr bool _S_copy_assign =
-         _S_copy_ctor && _S_move_ctor
+         _S_copy_ctor
          && (is_copy_assignable_v<_Types> && ...);
       static constexpr bool _S_move_assign =
          _S_move_ctor
@@ -613,7 +613,7 @@ namespace __variant
                          __variant::__get<__rhs_index>(*this);
                        if constexpr (is_same_v<
                                      remove_reference_t<decltype(__this_mem)>,
-                                     remove_reference_t<decltype(__rhs_mem)>>)
+                                     __remove_cvref_t<decltype(__rhs_mem)>>)
                          __this_mem = __rhs_mem;
                      }
                  }

diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index b67c98adf4a51219c398dfe684b1d371ff96188c..5cc2a9460a904fcc1c04e8f7c43251a03b328453 100644 (file)
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -63,6 +63,15 @@ struct MoveCtorOnly
 struct MoveCtorAndSwapOnly : MoveCtorOnly { };
 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
 {
   nonliteral() { }
@@ -81,6 +90,7 @@ void default_ctor()
   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<string, AllDeleted>>);
+  static_assert(is_default_constructible_v<variant<DeletedMoves>>);
 
   static_assert(noexcept(variant<int>()));
   static_assert(!noexcept(variant<Empty>()));
@@ -93,6 +103,7 @@ void copy_ctor()
   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<std::string>>);
+  static_assert(is_trivially_copy_constructible_v<variant<DeletedMoves>>);
 
   {
     variant<int> a;
@@ -116,6 +127,7 @@ void move_ctor()
 {
   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<int, DeletedMoves>>); // uses copy ctor
   static_assert(is_trivially_move_constructible_v<variant<int>>);
   static_assert(!is_trivially_move_constructible_v<variant<std::string>>);
   static_assert(!noexcept(variant<int, Empty>(declval<variant<int, Empty>>())));
@@ -157,6 +169,7 @@ void copy_assign()
   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<string>>);
+  static_assert(is_trivially_copy_assignable_v<variant<DeletedMoves>>);
   {
     variant<Empty> a;
     static_assert(!noexcept(a = a));
@@ -171,6 +184,7 @@ void move_assign()
 {
   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<int, DeletedMoves>>); // uses copy assignment
   static_assert(is_trivially_move_assignable_v<variant<int>>);
   static_assert(!is_trivially_move_assignable_v<variant<string>>);
   {

diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc
index 3ca375d374e592e8f3dd6c32835ed627fe9c5f15..c0f48432ca14dcebdb6c19806a67cc11e0cfd90f 100644 (file)
--- a/libstdc++-v3/testsuite/20_util/variant/run.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/run.cc
@@ -57,6 +57,15 @@ struct AlwaysThrow
   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()
 {
   variant<monostate, string> v;
@@ -80,6 +89,12 @@ void move_ctor()
   VERIFY(holds_alternative<string>(u));
   VERIFY(get<string>(u) == "a");
   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()
@@ -137,6 +152,13 @@ void move_assign()
   VERIFY(holds_alternative<string>(u));
   VERIFY(get<string>(u) == "a");
   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()