Fix std::variant regression caused by never-valueless optimization
authorJonathan Wakely <jwakely@redhat.com>
Tue, 23 Apr 2019 09:55:33 +0000 (10:55 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Tue, 23 Apr 2019 09:55:33 +0000 (10:55 +0100)
commit47a468bdbe67a2202f470900de27c5dc98a7e3e0
treec5618522eb97e1abea03c4452fb2b69c07e1a324
parentbe46043e07689050ba4522e907db618d26fc1bb8
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
libstdc++-v3/ChangeLog
libstdc++-v3/include/bits/basic_string.h
libstdc++-v3/include/std/variant
libstdc++-v3/testsuite/20_util/variant/compile.cc
libstdc++-v3/testsuite/20_util/variant/run.cc