diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index e2038875220..a34428f762b 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,34 @@ +2019-04-05 Jonathan Wakely + + PR libstdc++/87431 (again) + * include/bits/basic_string.h (__variant::_Never_valueless_alt): + Define partial specialization for basic_string. + * include/bits/shared_ptr.h (_Never_valueless_alt): Likewise for + shared_ptr and weak_ptr. + * include/bits/std_function.h (_Never_valueless_alt): Likewise for + function. + * include/bits/stl_vector.h (_Never_valueless_alt): Likewise for + vector. + * include/bits/unique_ptr.h (_Never_valueless_alt): Likewise for + unique_ptr. + * include/debug/vector (_Never_valueless_alt): Likewise for debug + vector. + * include/std/any (_Never_valueless_alt): Define explicit + specialization for any. + * include/std/variant (_Never_valueless_alt): Define primary template. + (__never_valueless): Use _Never_valueless_alt instead of + is_trivially_copyable. + (variant::emplace(Args&&...)): Add special case for non-throwing + initializations to avoid try-catch overhead. Add special case for + scalars produced by potentially-throwing conversions. Use + _Never_valueless_alt instead of is_trivially_copyable for the + remaining strong exception-safety cases. + (variant::emplace(initializer_list, Args&&...)): Likewise. + * testsuite/20_util/variant/87431.cc: Run both test functions. + * testsuite/20_util/variant/exception_safety.cc: New test. + * testsuite/20_util/variant/run.cc: Use pmr::string instead of string, + so the variant becomes valueless. + 2019-04-03 Jonathan Wakely PR libstdc++/85184 diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 4d0894be99b..20a56277a57 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -6800,7 +6800,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct __is_fast_hash> : std::false_type { }; -#if __cplusplus > 201103L +#if __cplusplus >= 201402L #define __cpp_lib_string_udls 201304 @@ -6843,7 +6843,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } // inline namespace string_literals } // inline namespace literals -#endif // __cplusplus > 201103L +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // basic_string into a variant, but only if move assignment cannot throw. + template + struct _Never_valueless_alt> + : std::is_nothrow_move_assignable> + { }; + } // namespace __detail::__variant +#endif // C++17 +#endif // C++14 _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index ee815f0d0a1..2d53478f1f4 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -732,6 +732,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // @} group pointer_abstractions +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // shared_ptr into a variant. + template + struct _Never_valueless_alt> + : std::true_type + { }; + + // Provide the strong exception-safety guarantee when emplacing a + // weak_ptr into a variant. + template + struct _Never_valueless_alt> + : std::true_type + { }; + } // namespace __detail::__variant +#endif // C++17 + _GLIBCXX_END_NAMESPACE_VERSION } // namespace diff --git a/libstdc++-v3/include/bits/std_function.h b/libstdc++-v3/include/bits/std_function.h index caa0fd816af..b70ed564d11 100644 --- a/libstdc++-v3/include/bits/std_function.h +++ b/libstdc++-v3/include/bits/std_function.h @@ -787,9 +787,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION swap(function<_Res(_Args...)>& __x, function<_Res(_Args...)>& __y) noexcept { __x.swap(__y); } +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // function into a variant. + template + struct _Never_valueless_alt> + : std::true_type + { }; + } // namespace __detail::__variant +#endif // C++17 + _GLIBCXX_END_NAMESPACE_VERSION } // namespace std #endif // C++11 - #endif // _GLIBCXX_STD_FUNCTION_H diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h index 10bf4fac62e..dd9382d254d 100644 --- a/libstdc++-v3/include/bits/stl_vector.h +++ b/libstdc++-v3/include/bits/stl_vector.h @@ -1938,6 +1938,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { __x.swap(__y); } _GLIBCXX_END_NAMESPACE_CONTAINER + +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // vector into a variant, but only if move assignment cannot throw. + template + struct _Never_valueless_alt<_GLIBCXX_STD_C::vector<_Tp, _Alloc>> + : std::is_nothrow_move_assignable<_GLIBCXX_STD_C::vector<_Tp, _Alloc>> + { }; + } // namespace __detail::__variant +#endif // C++17 + _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h index 61a3ef05460..963e4eb7d44 100644 --- a/libstdc++-v3/include/bits/unique_ptr.h +++ b/libstdc++-v3/include/bits/unique_ptr.h @@ -866,6 +866,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // @} group pointer_abstractions +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // unique_ptr into a variant. + template + struct _Never_valueless_alt> + : std::true_type + { }; + } // namespace __detail::__variant +#endif // C++17 + _GLIBCXX_END_NAMESPACE_VERSION } // namespace diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector index 6d9536ee73a..56b5e140fd3 100644 --- a/libstdc++-v3/include/debug/vector +++ b/libstdc++-v3/include/debug/vector @@ -801,6 +801,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Sequence, std::random_access_iterator_tag>& __it) { return std::__niter_base(__it.base()); } +#if __cplusplus >= 201703L + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing a + // vector into a variant, but only if move assignment cannot throw. + template + struct _Never_valueless_alt<__debug::vector<_Tp, _Alloc>> + : std::is_nothrow_move_assignable<__debug::vector<_Tp, _Alloc>> + { }; + } // namespace __detail::__variant +#endif // C++17 + _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 385a99ce744..b0553dccf22 100644 --- a/libstdc++-v3/include/std/any +++ b/libstdc++-v3/include/std/any @@ -614,9 +614,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// @} + namespace __detail::__variant + { + template struct _Never_valueless_alt; // see + + // Provide the strong exception-safety guarantee when emplacing an + // any into a variant. + template<> + struct _Never_valueless_alt + : std::true_type + { }; + } // namespace __detail::__variant + _GLIBCXX_END_NAMESPACE_VERSION } // namespace std -#endif // C++14 - +#endif // C++17 #endif // _GLIBCXX_ANY diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index a21ef3005cf..e52aa403009 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -327,11 +327,31 @@ namespace __variant _Variadic_union<_Rest...> _M_rest; }; + // _Never_valueless_alt is true for variant alternatives that can + // always be placed in a variant without it becoming valueless. + + // For suitably-small, trivially copyable types we can create temporaries + // on the stack and then memcpy them into place. + template + struct _Never_valueless_alt + : __and_, is_trivially_copyable<_Tp>> + { }; + + // 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. + // Whether _Never_valueless_alt is true or not affects the ABI of a + // variant using that alternative, so we can't change the value later! + + // True if every alternative in _Types... can be emplaced in a variant + // without it becoming valueless. If this is true, variant<_Types...> + // can never be valueless, which enables some minor optimizations. template - constexpr bool __never_valueless() - { - return (is_trivially_copyable_v<_Types> && ...); - } + constexpr bool __never_valueless() + { + return (_Never_valueless_alt<_Types>::value && ...); + } // Defines index and the dtor, possibly trivial. template @@ -776,19 +796,19 @@ namespace __variant { return __v._M_storage(); } template - struct _Extra_visit_slot_needed - { - template struct _Variant_never_valueless; + struct _Extra_visit_slot_needed + { + template struct _Variant_never_valueless; - template - struct _Variant_never_valueless> - : bool_constant<__never_valueless<_Types...>()> {}; + template + struct _Variant_never_valueless> + : bool_constant<__never_valueless<_Types...>()> {}; - static constexpr bool value = - (is_same_v<_Maybe_variant_cookie, __variant_cookie> - || is_same_v<_Maybe_variant_cookie, __variant_idx_cookie>) - && !_Variant_never_valueless<__remove_cvref_t<_Variant>>::value; - }; + static constexpr bool value = + (is_same_v<_Maybe_variant_cookie, __variant_cookie> + || is_same_v<_Maybe_variant_cookie, __variant_idx_cookie>) + && !_Variant_never_valueless<__remove_cvref_t<_Variant>>::value; + }; // Used for storing multi-dimensional vtable. template @@ -1329,31 +1349,45 @@ namespace __variant static_assert(_Np < sizeof...(_Types), "The index should be in [0, number of alternatives)"); using type = variant_alternative_t<_Np, variant>; - // If constructing the value can throw but move assigning it can't, - // construct in a temporary and then move assign from it. This gives - // the strong exception safety guarantee, ensuring we never become - // valueless. - if constexpr (is_trivially_copyable_v - && !is_nothrow_constructible_v) + // Provide the strong exception-safety guarantee when possible, + // to avoid becoming valueless. + if constexpr (is_nothrow_constructible_v) { - // If move assignment cannot throw then we can provide the - // strong exception safety guarantee, and never become valueless. + this->_M_reset(); + __variant_construct_by_index<_Np>(*this, + std::forward<_Args>(__args)...); + } + else if constexpr (is_scalar_v) + { + // This might invoke a potentially-throwing conversion operator: + const type __tmp(std::forward<_Args>(__args)...); + // But these steps won't throw: + this->_M_reset(); + __variant_construct_by_index<_Np>(*this, __tmp); + } + else if constexpr (__detail::__variant::_Never_valueless_alt()) + { + // This construction might throw: variant __tmp(in_place_index<_Np>, std::forward<_Args>(__args)...); + // But _Never_valueless_alt means this won't: *this = std::move(__tmp); - return std::get<_Np>(*this); } - - this->_M_reset(); - __try + else { - __variant_construct_by_index<_Np>(*this, - std::forward<_Args>(__args)...); - } - __catch (...) - { - this->_M_index = variant_npos; - __throw_exception_again; + // This case only provides the basic exception-safety guarantee, + // i.e. the variant can become valueless. + this->_M_reset(); + __try + { + __variant_construct_by_index<_Np>(*this, + std::forward<_Args>(__args)...); + } + __catch (...) + { + this->_M_index = variant_npos; + __throw_exception_again; + } } return std::get<_Np>(*this); } @@ -1367,28 +1401,39 @@ namespace __variant static_assert(_Np < sizeof...(_Types), "The index should be in [0, number of alternatives)"); using type = variant_alternative_t<_Np, variant>; - if constexpr (is_trivially_copyable_v - && !is_nothrow_constructible_v, - _Args...>) + // Provide the strong exception-safety guarantee when possible, + // to avoid becoming valueless. + if constexpr (is_nothrow_constructible_v&, + _Args...>) { - // If move assignment cannot throw then we can provide the - // strong exception safety guarantee, and never become valueless. + this->_M_reset(); + __variant_construct_by_index<_Np>(*this, __il, + std::forward<_Args>(__args)...); + } + else if constexpr (__detail::__variant::_Never_valueless_alt()) + { + // This construction might throw: variant __tmp(in_place_index<_Np>, __il, std::forward<_Args>(__args)...); + // But _Never_valueless_alt means this won't: *this = std::move(__tmp); - return std::get<_Np>(*this); } - - this->_M_reset(); - __try + else { - __variant_construct_by_index<_Np>(*this, __il, - std::forward<_Args>(__args)...); - } - __catch (...) - { - this->_M_index = variant_npos; - __throw_exception_again; + // This case only provides the basic exception-safety guarantee, + // i.e. the variant can become valueless. + this->_M_reset(); + __try + { + __variant_construct_by_index<_Np>(*this, __il, + std::forward<_Args>(__args)...); + } + __catch (...) + { + this->_M_index = variant_npos; + __throw_exception_again; + } } return std::get<_Np>(*this); } diff --git a/libstdc++-v3/testsuite/20_util/variant/87431.cc b/libstdc++-v3/testsuite/20_util/variant/87431.cc index 8c5c4a78154..bf92564cf92 100644 --- a/libstdc++-v3/testsuite/20_util/variant/87431.cc +++ b/libstdc++-v3/testsuite/20_util/variant/87431.cc @@ -68,4 +68,5 @@ int main() { test01(); + test02(); } diff --git a/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc b/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc new file mode 100644 index 00000000000..7e1b0f3bed1 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc @@ -0,0 +1,178 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++17" } +// { dg-do run { target c++17 } } + +#include +#include +#include +#include +#include +#include +#include +#include + +void +test01() +{ +#if _GLIBCXX_USE_CXX11_ABI + std::variant> v(1); + VERIFY( v.index() == 0 ); + + try + { + std::pmr::string s = "how long is a piece of SSO string?"; + v.emplace<1>(s, std::pmr::null_memory_resource()); + VERIFY( false ); + } + catch(const std::bad_alloc&) + { + VERIFY( v.valueless_by_exception() ); + } + + v.emplace<0>(2); + VERIFY( v.index() == 0 ); + + try + { + v.emplace<2>({1, 2, 3}, std::pmr::null_memory_resource()); + VERIFY( false ); + } + catch(const std::bad_alloc&) + { + VERIFY( v.valueless_by_exception() ); + } +#endif +} + +void +test02() +{ + struct X + { + X(int i) : i(1) { if (i > 2) throw 3; } + X(std::initializer_list l) : i(2) { if (l.size() > 2) throw 3; } + int i; + }; + static_assert( std::is_trivially_copyable_v ); + + std::variant v(111); + VERIFY( v.index() == 1 ); + + try + { + v.emplace(3); + VERIFY( false ); + } + catch(int) + { + VERIFY( !v.valueless_by_exception() ); + VERIFY( v.index() == 1 ); + VERIFY( std::get(v) == 111 ); + } + + v.emplace(1); + VERIFY( v.index() == 2 ); + VERIFY( std::get(v).i == 1 ); + + try + { + v.emplace(3); + VERIFY( false ); + } + catch(int) + { + VERIFY( !v.valueless_by_exception() ); + VERIFY( v.index() == 2 ); + VERIFY( std::get(v).i == 1 ); + } + + try + { + v.emplace({1, 2, 3}); + VERIFY( false ); + } + catch(int) + { + VERIFY( !v.valueless_by_exception() ); + VERIFY( v.index() == 2 ); + VERIFY( std::get(v).i == 1 ); + } +} + +template + bool bad_emplace(V& v) + { + struct X { + operator T() const { throw 1; } + }; + + const auto index = v.index(); + + try + { + if (std::is_same_v) + { + // Need to test std::any differently, because emplace(X{}) + // would create a std::any with a contained X, instead of using + // X::operator any() to convert to std::any. + struct ThrowOnCopy { + ThrowOnCopy() { } + ThrowOnCopy(const ThrowOnCopy&) { throw 1; } + } t; + v.template emplace(t); + } + else + v.template emplace(X{}); + } + catch (int) + { + return v.index() == index; + } + return false; + } + +void +test03() +{ + struct TriviallyCopyable { int i = 0; }; + + std::variant, + std::string, std::vector, std::function, std::any, + std::shared_ptr, std::weak_ptr, std::unique_ptr> v(1); + VERIFY( v.index() == 1 ); + + VERIFY( bad_emplace(v) ); + VERIFY( bad_emplace(v) ); + VERIFY( bad_emplace>(v) ); + VERIFY( bad_emplace(v) ); + VERIFY( bad_emplace>(v) ); + VERIFY( bad_emplace>(v) ); + VERIFY( bad_emplace(v) ); + VERIFY( bad_emplace>(v) ); + VERIFY( bad_emplace>(v) ); + VERIFY( bad_emplace>(v) ); +} + +int +main() +{ + test01(); + test02(); + test03(); +} diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc index 7ee9b08c2ce..3ca375d374e 100644 --- a/libstdc++-v3/testsuite/20_util/variant/run.cc +++ b/libstdc++-v3/testsuite/20_util/variant/run.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include using namespace std; @@ -376,7 +377,7 @@ void test_visit() void test_hash() { - unordered_set> s; + unordered_set> s; VERIFY(s.emplace(3).second); VERIFY(s.emplace("asdf").second); VERIFY(s.emplace().second); @@ -388,12 +389,12 @@ void test_hash() { struct A { - operator string() + operator pmr::string() { throw nullptr; } }; - variant v; + variant v; try { v.emplace<1>(A{});