Fix std::variant regression caused by never-valueless optimization

A regression was introduced by the recent changes to provide the strong
exception safety guarantee for "never valueless" types that have O(1),
non-throwing move assignment. The problematic code is:

  else if constexpr (__detail::__variant::_Never_valueless_alt<type>())
    {
      // This construction might throw:
      variant __tmp(in_place_index<_Np>, __il,
                    std::forward<_Args>(__args)...);
      // But _Never_valueless_alt<type> means this won't:
      *this = std::move(__tmp);
    }

When the variant is not assignable, the assignment is ill-formed, so
should not be attempted. When the variant has a copy assignment operator
but not a move assignment operator, the assignment performs a copy
assignment and that could throw, so should not be attempted.

The solution is to only take that branch when the variant has a move
assignment operator, which is determined by the _Traits::_S_move_assign
constant. When that is false the strong exception safety guarantee is
not possible, and so the __never_valueless function should also depend
on _S_move_assign.

While testing the fixes for this I noticed that the partial
specialization _Never_valueless_alt<basic_string<C,T,A>> incorrectly
assumed that is_nothrow_move_constructible<basic_string<C,T,A>> is
always true, but that's wrong for fully-dynamic COW strings. Fix the
partial specialization, and improve the comment describing
_Never_valueless_alt to be clear it depends on move construction as well
as move assignment.

Finally, I also observed that _Variant_storage<false, T...>::_M_valid()
was not taking advantage of the __never_valueless<T...>() function to
avoid a runtime check. Only the _Variant_storage<true, T...>::_M_valid()
function was using __never_valueless. That is also fixed.

	PR libstdc++/87431
	* include/bits/basic_string.h (_Never_valueless_alt): Make partial
	specialization also depend on is_nothrow_move_constructible.
	* include/std/variant (__detail::__variant::__never_valueless()):
	Only true if the variant would have a move assignment operator.
	(__detail::__variant::_Variant_storage<false, T...>::_M_valid()):
	Check __never_valueless<T...>().
	(variant::emplace): Only perform non-throwing move assignments
	for never-valueless alternatives if the variant has a move assignment
	operator.
	* testsuite/20_util/variant/compile.cc: Check that never-valueless
	types can be emplaced into non-assignable variants.
	* testsuite/20_util/variant/run.cc: Check that never-valueless types
	don't get copied when emplaced into non-assignable variants.

From-SVN: r270502

5 files changed, 81 insertions, 8 deletions

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 9dad28f..0949c94 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,20 @@
 2019-04-23  Jonathan Wakely  <jwakely@redhat.com>
 
+	PR libstdc++/87431
+	* include/bits/basic_string.h (_Never_valueless_alt): Make partial
+	specialization also depend on is_nothrow_move_constructible.
+	* include/std/variant (__detail::__variant::__never_valueless()):
+	Only true if the variant would have a move assignment operator.
+	(__detail::__variant::_Variant_storage<false, T...>::_M_valid()):
+	Check __never_valueless<T...>().
+	(variant::emplace): Only perform non-throwing move assignments
+	for never-valueless alternatives if the variant has a move assignment
+	operator.
+	* testsuite/20_util/variant/compile.cc: Check that never-valueless
+	types can be emplaced into non-assignable variants.
+	* testsuite/20_util/variant/run.cc: Check that never-valueless types
+	don't get copied when emplaced into non-assignable variants.
+
 	* include/std/variant (__detail::__variant::__ref_cast): Remove
 	unused function.
 	(__detail::__variant::_Uninitialized::_M_get)
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 20a5627..9225369 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -6849,10 +6849,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename> struct _Never_valueless_alt; // see <variant>
 
     // Provide the strong exception-safety guarantee when emplacing a
-    // basic_string into a variant, but only if move assignment cannot throw.
+    // basic_string into a variant, but only if moving the string cannot throw.
     template<typename _Tp, typename _Traits, typename _Alloc>
       struct _Never_valueless_alt<std::basic_string<_Tp, _Traits, _Alloc>>
-      : std::is_nothrow_move_assignable<std::basic_string<_Tp, _Traits, _Alloc>>
+      : __and_<
+	is_nothrow_move_constructible<std::basic_string<_Tp, _Traits, _Alloc>>,
+	is_nothrow_move_assignable<std::basic_string<_Tp, _Traits, _Alloc>>
+	>::type
       { };
   }  // namespace __detail::__variant
 #endif // C++17
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 08378ee..b71bb02 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -340,9 +340,9 @@ namespace __variant
     { };
 
   // Specialize _Never_valueless_alt for other types which have a
-  // non-throwing and cheap move assignment operator, so that emplacing
-  // the type will provide the strong exception-safety guarantee,
-  // by creating and moving a temporary.
+  // non-throwing and cheap move construction and move assignment operator,
+  // so that emplacing the type will provide the strong exception-safety
+  // guarantee, by creating and moving a temporary.
   // Whether _Never_valueless_alt<T> is true or not affects the ABI of a
   // variant using that alternative, so we can't change the value later!
 
@@ -352,7 +352,8 @@ namespace __variant
   template <typename... _Types>
     constexpr bool __never_valueless()
     {
-      return (_Never_valueless_alt<_Types>::value && ...);
+      return _Traits<_Types...>::_S_move_assign
+	&& (_Never_valueless_alt<_Types>::value && ...);
     }
 
   // Defines index and the dtor, possibly trivial.
@@ -407,6 +408,8 @@ namespace __variant
       constexpr bool
       _M_valid() const noexcept
       {
+	if constexpr (__never_valueless<_Types...>())
+	  return true;
 	return this->_M_index != __index_type(variant_npos);
       }
 
@@ -1428,7 +1431,8 @@ namespace __variant
 	      this->_M_reset();
 	      __variant_construct_by_index<_Np>(*this, __tmp);
 	    }
-	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>())
+	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>()
+	      && _Traits::_S_move_assign)
 	    {
 	      // This construction might throw:
 	      variant __tmp(in_place_index<_Np>,
@@ -1474,7 +1478,8 @@ namespace __variant
 	      __variant_construct_by_index<_Np>(*this, __il,
 		  std::forward<_Args>(__args)...);
 	    }
-	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>())
+	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>()
+	      && _Traits::_S_move_assign)
 	    {
 	      // This construction might throw:
 	      variant __tmp(in_place_index<_Np>, __il,
diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 5cc2a94..afd593d 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -479,6 +479,20 @@ void test_emplace()
   static_assert(has_index_emplace<variant<int>, 0>(0));
   static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0));
   static_assert(!has_index_emplace<variant<AllDeleted>, 0>(0));
+  static_assert(has_type_emplace<variant<int, AllDeleted>, int>(0));
+  static_assert(has_index_emplace<variant<int, AllDeleted>, 0>(0));
+  static_assert(has_type_emplace<variant<int, vector<int>, AllDeleted>, vector<int>>(0));
+  static_assert(has_index_emplace<variant<int, vector<int>, AllDeleted>, 1>(0));
+
+  // The above tests only check the emplace members are available for
+  // overload resolution. The following odr-uses will instantiate them:
+  variant<int, vector<int>, AllDeleted> v;
+  v.emplace<0>(1);
+  v.emplace<int>(1);
+  v.emplace<1>(1, 1);
+  v.emplace<vector<int>>(1, 1);
+  v.emplace<1>({1, 2, 3, 4});
+  v.emplace<vector<int>>({1, 2, 3, 4});
 }
 
 void test_triviality()
diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc
index c0f4843..ec1e868 100644
--- a/libstdc++-v3/testsuite/20_util/variant/run.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/run.cc
@@ -23,6 +23,7 @@
 #include <vector>
 #include <unordered_set>
 #include <memory_resource>
+#include <ext/throw_allocator.h>
 #include <testsuite_hooks.h>
 
 using namespace std;
@@ -254,6 +255,41 @@ void emplace()
     VERIFY(&v.emplace<0>({1,2,3}) == &std::get<0>(v));
     VERIFY(&v.emplace<vector<int>>({1,2,3}) == &std::get<vector<int>>(v));
   }
+
+  {
+    // Ensure no copies of the vector are made, only moves.
+    // See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87431#c21
+
+    // static_assert(__detail::__variant::_Never_valueless_alt<vector<AlwaysThrow>>::value);
+    variant<int, DeletedMoves, vector<AlwaysThrow>> v;
+    v.emplace<2>(1);
+    v.emplace<vector<AlwaysThrow>>(1);
+    v.emplace<0>(0);
+
+    // To test the emplace(initializer_list<U>, Args&&...) members we
+    // can't use AlwaysThrow because elements in an initialier_list
+    // are always copied. Use throw_allocator instead.
+    using Vector = vector<int, __gnu_cxx::throw_allocator_limit<int>>;
+    // static_assert(__detail::__variant::_Never_valueless_alt<Vector>::value);
+    variant<int, DeletedMoves, Vector> vv;
+    Vector::allocator_type::set_limit(1);
+    vv.emplace<2>(1, 1);
+    Vector::allocator_type::set_limit(1);
+    vv.emplace<Vector>(1, 1);
+    Vector::allocator_type::set_limit(1);
+    vv.emplace<0>(0);
+    Vector::allocator_type::set_limit(1);
+    vv.emplace<2>({1, 2, 3});
+    Vector::allocator_type::set_limit(1);
+    vv.emplace<Vector>({1, 2, 3, 4});
+    try {
+      Vector::allocator_type::set_limit(0);
+      vv.emplace<2>(1, 1);
+      VERIFY(false);
+    } catch (__gnu_cxx::forced_error) {
+    }
+    VERIFY(vv.valueless_by_exception());
+  }
 }
 
 void test_get()