From 705037247447f44826bd6fc2c777c69237fcef39 Mon Sep 17 00:00:00 2001 From: Tim Shen Date: Tue, 27 Jun 2017 18:19:03 +0000 Subject: [PATCH] re PR libstdc++/80187 (C++ variant should be trivially copy constructible if possible) PR libstdc++/80187 * include/std/variant (variant::variant, variant::~variant, variant::operator=): Implement triviality forwarding for four special member functions. * testsuite/20_util/variant/compile.cc: Tests. From-SVN: r249706 --- libstdc++-v3/ChangeLog | 8 + libstdc++-v3/include/std/variant | 312 +++++++++++++----- .../testsuite/20_util/variant/compile.cc | 103 +++++- 3 files changed, 341 insertions(+), 82 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 1d31df9dce4..770b5789da7 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,11 @@ +2017-06-27 Tim Shen + + PR libstdc++/80187 + * include/std/variant (variant::variant, variant::~variant, + variant::operator=): Implement triviality forwarding for four + special member functions. + * testsuite/20_util/variant/compile.cc: Tests. + 2017-06-27 Jonathan Wakely PR libstdc++/81221 diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index e5fe9f927c1..06646f6b798 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -295,6 +295,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __variant::__ref_cast<_Tp>(__t)); } + template + struct _Traits + { + static constexpr bool _S_default_ctor = + is_default_constructible_v::type>; + static constexpr bool _S_copy_ctor = + (is_copy_constructible_v<_Types> && ...); + static constexpr bool _S_move_ctor = + (is_move_constructible_v<_Types> && ...); + static constexpr bool _S_copy_assign = + _S_copy_ctor && _S_move_ctor + && (is_copy_assignable_v<_Types> && ...); + static constexpr bool _S_move_assign = + _S_move_ctor + && (is_move_assignable_v<_Types> && ...); + + static constexpr bool _S_trivial_dtor = + (is_trivially_destructible_v<_Types> && ...); + static constexpr bool _S_trivial_copy_ctor = + (is_trivially_copy_constructible_v<_Types> && ...); + static constexpr bool _S_trivial_move_ctor = + (is_trivially_move_constructible_v<_Types> && ...); + static constexpr bool _S_trivial_copy_assign = + _S_trivial_dtor && (is_trivially_copy_assignable_v<_Types> && ...); + static constexpr bool _S_trivial_move_assign = + _S_trivial_dtor && (is_trivially_move_assignable_v<_Types> && ...); + + // The following nothrow traits are for non-trivial SMFs. Trivial SMFs + // are always nothrow. + static constexpr bool _S_nothrow_default_ctor = + is_nothrow_default_constructible_v< + typename _Nth_type<0, _Types...>::type>; + static constexpr bool _S_nothrow_copy_ctor = false; + static constexpr bool _S_nothrow_move_ctor = + (is_nothrow_move_constructible_v<_Types> && ...); + static constexpr bool _S_nothrow_copy_assign = false; + static constexpr bool _S_nothrow_move_assign = + _S_nothrow_move_ctor && (is_nothrow_move_assignable_v<_Types> && ...); + }; + // Defines members and ctors. template union _Variadic_union { }; @@ -360,6 +400,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION ~_Variant_storage() { _M_reset(); } + void* + _M_storage() const + { + return const_cast(static_cast( + std::addressof(_M_u))); + } + + constexpr bool + _M_valid() const noexcept + { + return this->_M_index != __index_type(variant_npos); + } + _Variadic_union<_Types...> _M_u; using __index_type = __select_index<_Types...>; __index_type _M_index; @@ -379,59 +432,108 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _M_reset() { _M_index = variant_npos; } + void* + _M_storage() const + { + return const_cast(static_cast( + std::addressof(_M_u))); + } + + constexpr bool + _M_valid() const noexcept + { + return this->_M_index != __index_type(variant_npos); + } + _Variadic_union<_Types...> _M_u; using __index_type = __select_index<_Types...>; __index_type _M_index; }; - // Helps SFINAE on special member functions. Otherwise it can live in variant - // class. template - struct _Variant_base : - _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...), - _Types...> - { - using _Storage = - _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...), - _Types...>; + using _Variant_storage_alias = + _Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>; - constexpr - _Variant_base() - noexcept(is_nothrow_default_constructible_v< - variant_alternative_t<0, variant<_Types...>>>) - : _Variant_base(in_place_index<0>) { } + // The following are (Copy|Move) (ctor|assign) layers for forwarding + // triviality and handling non-trivial SMF behaviors. + + template + struct _Copy_ctor_base : _Variant_storage_alias<_Types...> + { + using _Base = _Variant_storage_alias<_Types...>; + using _Base::_Base; - _Variant_base(const _Variant_base& __rhs) + _Copy_ctor_base(const _Copy_ctor_base& __rhs) + noexcept(_Traits<_Types...>::_S_nothrow_copy_ctor) { if (__rhs._M_valid()) { static constexpr void (*_S_vtable[])(void*, void*) = { &__erased_ctor<_Types&, const _Types&>... }; - _S_vtable[__rhs._M_index](_M_storage(), __rhs._M_storage()); + _S_vtable[__rhs._M_index](this->_M_storage(), __rhs._M_storage()); this->_M_index = __rhs._M_index; } } - _Variant_base(_Variant_base&& __rhs) - noexcept((is_nothrow_move_constructible_v<_Types> && ...)) + _Copy_ctor_base(_Copy_ctor_base&&) = default; + _Copy_ctor_base& operator=(const _Copy_ctor_base&) = default; + _Copy_ctor_base& operator=(_Copy_ctor_base&&) = default; + }; + + template + struct _Copy_ctor_base : _Variant_storage_alias<_Types...> + { + using _Base = _Variant_storage_alias<_Types...>; + using _Base::_Base; + }; + + template + using _Copy_ctor_alias = + _Copy_ctor_base<_Traits<_Types...>::_S_trivial_copy_ctor, _Types...>; + + template + struct _Move_ctor_base : _Copy_ctor_alias<_Types...> + { + using _Base = _Copy_ctor_alias<_Types...>; + using _Base::_Base; + + _Move_ctor_base(_Move_ctor_base&& __rhs) + noexcept(_Traits<_Types...>::_S_nothrow_move_ctor) { if (__rhs._M_valid()) { static constexpr void (*_S_vtable[])(void*, void*) = { &__erased_ctor<_Types&, _Types&&>... }; - _S_vtable[__rhs._M_index](_M_storage(), __rhs._M_storage()); + _S_vtable[__rhs._M_index](this->_M_storage(), __rhs._M_storage()); this->_M_index = __rhs._M_index; } } - template - constexpr explicit - _Variant_base(in_place_index_t<_Np> __i, _Args&&... __args) - : _Storage(__i, std::forward<_Args>(__args)...) - { } + _Move_ctor_base(const _Move_ctor_base&) = default; + _Move_ctor_base& operator=(const _Move_ctor_base&) = default; + _Move_ctor_base& operator=(_Move_ctor_base&&) = default; + }; + + template + struct _Move_ctor_base : _Copy_ctor_alias<_Types...> + { + using _Base = _Copy_ctor_alias<_Types...>; + using _Base::_Base; + }; + + template + using _Move_ctor_alias = + _Move_ctor_base<_Traits<_Types...>::_S_trivial_move_ctor, _Types...>; + + template + struct _Copy_assign_base : _Move_ctor_alias<_Types...> + { + using _Base = _Move_ctor_alias<_Types...>; + using _Base::_Base; - _Variant_base& - operator=(const _Variant_base& __rhs) + _Copy_assign_base& + operator=(const _Copy_assign_base& __rhs) + noexcept(_Traits<_Types...>::_S_nothrow_copy_assign) { if (this->_M_index == __rhs._M_index) { @@ -439,16 +541,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { static constexpr void (*_S_vtable[])(void*, void*) = { &__erased_assign<_Types&, const _Types&>... }; - _S_vtable[__rhs._M_index](_M_storage(), __rhs._M_storage()); + _S_vtable[__rhs._M_index](this->_M_storage(), __rhs._M_storage()); } } else { - _Variant_base __tmp(__rhs); - this->~_Variant_base(); + _Copy_assign_base __tmp(__rhs); + this->~_Copy_assign_base(); __try { - ::new (this) _Variant_base(std::move(__tmp)); + ::new (this) _Copy_assign_base(std::move(__tmp)); } __catch (...) { @@ -460,12 +562,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return *this; } - void _M_destructive_move(_Variant_base&& __rhs) + _Copy_assign_base(const _Copy_assign_base&) = default; + _Copy_assign_base(_Copy_assign_base&&) = default; + _Copy_assign_base& operator=(_Copy_assign_base&&) = default; + }; + + template + struct _Copy_assign_base : _Move_ctor_alias<_Types...> + { + using _Base = _Move_ctor_alias<_Types...>; + using _Base::_Base; + }; + + template + using _Copy_assign_alias = + _Copy_assign_base<_Traits<_Types...>::_S_trivial_copy_assign, + _Types...>; + + template + struct _Move_assign_base : _Copy_assign_alias<_Types...> + { + using _Base = _Copy_assign_alias<_Types...>; + using _Base::_Base; + + void _M_destructive_move(_Move_assign_base&& __rhs) { - this->~_Variant_base(); + this->~_Move_assign_base(); __try { - ::new (this) _Variant_base(std::move(__rhs)); + ::new (this) _Move_assign_base(std::move(__rhs)); } __catch (...) { @@ -474,40 +599,74 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } - _Variant_base& - operator=(_Variant_base&& __rhs) - noexcept((is_nothrow_move_constructible_v<_Types> && ...) - && (is_nothrow_move_assignable_v<_Types> && ...)) + _Move_assign_base& + operator=(_Move_assign_base&& __rhs) + noexcept(_Traits<_Types...>::_S_nothrow_move_assign) { if (this->_M_index == __rhs._M_index) { if (__rhs._M_valid()) { static constexpr void (*_S_vtable[])(void*, void*) = - { &__erased_assign<_Types&, _Types&&>... }; - _S_vtable[__rhs._M_index](_M_storage(), __rhs._M_storage()); + { &__erased_assign<_Types&, const _Types&>... }; + _S_vtable[__rhs._M_index](this->_M_storage(), __rhs._M_storage()); } } else { - _M_destructive_move(std::move(__rhs)); + _Move_assign_base __tmp(__rhs); + this->~_Move_assign_base(); + __try + { + ::new (this) _Move_assign_base(std::move(__tmp)); + } + __catch (...) + { + this->_M_index = variant_npos; + __throw_exception_again; + } } + __glibcxx_assert(this->_M_index == __rhs._M_index); return *this; } - void* - _M_storage() const - { - return const_cast(static_cast( - std::addressof(_Storage::_M_u))); - } + _Move_assign_base(const _Move_assign_base&) = default; + _Move_assign_base(_Move_assign_base&&) = default; + _Move_assign_base& operator=(const _Move_assign_base&) = default; + }; - constexpr bool - _M_valid() const noexcept - { - return this->_M_index != - typename _Storage::__index_type(variant_npos); - } + template + struct _Move_assign_base : _Copy_assign_alias<_Types...> + { + using _Base = _Copy_assign_alias<_Types...>; + using _Base::_Base; + }; + + template + using _Move_assign_alias = + _Move_assign_base<_Traits<_Types...>::_S_trivial_move_assign, + _Types...>; + + template + struct _Variant_base : _Move_assign_alias<_Types...> + { + using _Base = _Move_assign_alias<_Types...>; + + constexpr + _Variant_base() + noexcept(_Traits<_Types...>::_S_nothrow_default_ctor) + : _Variant_base(in_place_index<0>) { } + + template + constexpr explicit + _Variant_base(in_place_index_t<_Np> __i, _Args&&... __args) + : _Base(__i, std::forward<_Args>(__args)...) + { } + + _Variant_base(const _Variant_base&) = default; + _Variant_base(_Variant_base&&) = default; + _Variant_base& operator=(const _Variant_base&) = default; + _Variant_base& operator=(_Variant_base&&) = default; }; // For how many times does _Tp appear in _Tuple? @@ -882,16 +1041,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION class variant : private __detail::__variant::_Variant_base<_Types...>, private _Enable_default_constructor< - is_default_constructible_v< - variant_alternative_t<0, variant<_Types...>>>, variant<_Types...>>, + __detail::__variant::_Traits<_Types...>::_S_default_ctor, + variant<_Types...>>, private _Enable_copy_move< - (is_copy_constructible_v<_Types> && ...), - (is_copy_constructible_v<_Types> && ...) - && (is_move_constructible_v<_Types> && ...) - && (is_copy_assignable_v<_Types> && ...), - (is_move_constructible_v<_Types> && ...), - (is_move_constructible_v<_Types> && ...) - && (is_move_assignable_v<_Types> && ...), + __detail::__variant::_Traits<_Types...>::_S_copy_ctor, + __detail::__variant::_Traits<_Types...>::_S_copy_assign, + __detail::__variant::_Traits<_Types...>::_S_move_ctor, + __detail::__variant::_Traits<_Types...>::_S_move_assign, variant<_Types...>> { private: @@ -904,9 +1060,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION using _Base = __detail::__variant::_Variant_base<_Types...>; using _Default_ctor_enabler = - _Enable_default_constructor< - is_default_constructible_v< - variant_alternative_t<0, variant<_Types...>>>, variant<_Types...>>; + _Enable_default_constructor< + __detail::__variant::_Traits<_Types...>::_S_default_ctor, + variant<_Types...>>; template static constexpr bool @@ -933,12 +1089,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static constexpr size_t __index_of = __detail::__variant::__index_of_v<_Tp, _Types...>; + using _Traits = __detail::__variant::_Traits<_Types...>; + public: - constexpr variant() - noexcept(is_nothrow_default_constructible_v<__to_type<0>>) = default; - variant(const variant&) = default; - variant(variant&&) - noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = default; + variant() = default; + variant(const variant& __rhs) = default; + variant(variant&&) = default; + variant& operator=(const variant&) = default; + variant& operator=(variant&&) = default; + ~variant() = default; template, variant>>, @@ -947,7 +1106,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr variant(_Tp&& __t) noexcept(is_nothrow_constructible_v<__accepted_type<_Tp&&>, _Tp&&>) - : variant(in_place_index<__accepted_index<_Tp&&>>, std::forward<_Tp>(__t)) + : variant(in_place_index<__accepted_index<_Tp&&>>, + std::forward<_Tp>(__t)) { __glibcxx_assert(holds_alternative<__accepted_type<_Tp&&>>(*this)); } template>> constexpr explicit variant(in_place_type_t<_Tp>, _Args&&... __args) - : variant(in_place_index<__index_of<_Tp>>, std::forward<_Args>(__args)...) + : variant(in_place_index<__index_of<_Tp>>, + std::forward<_Args>(__args)...) { __glibcxx_assert(holds_alternative<_Tp>(*this)); } template && ...) - && (is_nothrow_move_assignable_v<_Types> && ...)) = default; - template enable_if_t<__exactly_once<__accepted_type<_Tp&&>> && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&> @@ -1089,7 +1243,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr size_t index() const noexcept { if (this->_M_index == - typename _Base::_Storage::__index_type(variant_npos)) + typename _Base::__index_type(variant_npos)) return variant_npos; return this->_M_index; } diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc index 06e8eb31ee8..e5f7538ba42 100644 --- a/libstdc++-v3/testsuite/20_util/variant/compile.cc +++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc @@ -88,10 +88,12 @@ void copy_ctor() { static_assert(is_copy_constructible_v>, ""); static_assert(!is_copy_constructible_v>, ""); + static_assert(is_trivially_copy_constructible_v>, ""); + static_assert(!is_trivially_copy_constructible_v>, ""); { variant a; - static_assert(!noexcept(variant(a)), ""); + static_assert(noexcept(variant(a)), ""); } { variant a; @@ -103,7 +105,7 @@ void copy_ctor() } { variant a; - static_assert(!noexcept(variant(a)), ""); + static_assert(noexcept(variant(a)), ""); } } @@ -111,6 +113,8 @@ void move_ctor() { static_assert(is_move_constructible_v>, ""); static_assert(!is_move_constructible_v>, ""); + static_assert(is_trivially_move_constructible_v>, ""); + static_assert(!is_trivially_move_constructible_v>, ""); static_assert(!noexcept(variant(declval>())), ""); static_assert(noexcept(variant(declval>())), ""); } @@ -148,13 +152,15 @@ void copy_assign() { static_assert(is_copy_assignable_v>, ""); static_assert(!is_copy_assignable_v>, ""); + static_assert(is_trivially_copy_assignable_v>, ""); + static_assert(!is_trivially_copy_assignable_v>, ""); { variant a; static_assert(!noexcept(a = a), ""); } { variant a; - static_assert(!noexcept(a = a), ""); + static_assert(noexcept(a = a), ""); } } @@ -162,6 +168,8 @@ void move_assign() { static_assert(is_move_assignable_v>, ""); static_assert(!is_move_assignable_v>, ""); + static_assert(is_trivially_move_assignable_v>, ""); + static_assert(!is_trivially_move_assignable_v>, ""); { variant a; static_assert(!noexcept(a = std::move(a)), ""); @@ -454,3 +462,92 @@ void test_emplace() static_assert(!has_type_emplace, AllDeleted>(0), ""); static_assert(!has_index_emplace, 0>(0), ""); } + +void test_triviality() +{ +#define TEST_TEMPLATE(DT, CC, MC, CA, MA, CC_VAL, MC_VAL, CA_VAL, MA_VAL) \ + { \ + struct A \ + { \ + ~A() DT; \ + A(const A&) CC; \ + A(A&&) MC; \ + A& operator=(const A&) CA; \ + A& operator=(A&&) MA; \ + }; \ + static_assert(CC_VAL == is_trivially_copy_constructible_v>, ""); \ + static_assert(MC_VAL == is_trivially_move_constructible_v>, ""); \ + static_assert(CA_VAL == is_trivially_copy_assignable_v>, ""); \ + static_assert(MA_VAL == is_trivially_move_assignable_v>, ""); \ + } + TEST_TEMPLATE(=default, =default, =default, =default, =default, true, true, true, true) + TEST_TEMPLATE(=default, =default, =default, =default, {}, true, true, true, false) + TEST_TEMPLATE(=default, =default, =default, {}, =default, true, true, false, true) + TEST_TEMPLATE(=default, =default, =default, {}, {}, true, true, false, false) + TEST_TEMPLATE(=default, =default, {}, =default, =default, true, false, true, true) + TEST_TEMPLATE(=default, =default, {}, =default, {}, true, false, true, false) + TEST_TEMPLATE(=default, =default, {}, {}, =default, true, false, false, true) + TEST_TEMPLATE(=default, =default, {}, {}, {}, true, false, false, false) + TEST_TEMPLATE(=default, {}, =default, =default, =default, false, true, true, true) + TEST_TEMPLATE(=default, {}, =default, =default, {}, false, true, true, false) + TEST_TEMPLATE(=default, {}, =default, {}, =default, false, true, false, true) + TEST_TEMPLATE(=default, {}, =default, {}, {}, false, true, false, false) + TEST_TEMPLATE(=default, {}, {}, =default, =default, false, false, true, true) + TEST_TEMPLATE(=default, {}, {}, =default, {}, false, false, true, false) + TEST_TEMPLATE(=default, {}, {}, {}, =default, false, false, false, true) + TEST_TEMPLATE(=default, {}, {}, {}, {}, false, false, false, false) + TEST_TEMPLATE( {}, =default, =default, =default, =default, false, false, false, false) + TEST_TEMPLATE( {}, =default, =default, =default, {}, false, false, false, false) + TEST_TEMPLATE( {}, =default, =default, {}, =default, false, false, false, false) + TEST_TEMPLATE( {}, =default, =default, {}, {}, false, false, false, false) + TEST_TEMPLATE( {}, =default, {}, =default, =default, false, false, false, false) + TEST_TEMPLATE( {}, =default, {}, =default, {}, false, false, false, false) + TEST_TEMPLATE( {}, =default, {}, {}, =default, false, false, false, false) + TEST_TEMPLATE( {}, =default, {}, {}, {}, false, false, false, false) + TEST_TEMPLATE( {}, {}, =default, =default, =default, false, false, false, false) + TEST_TEMPLATE( {}, {}, =default, =default, {}, false, false, false, false) + TEST_TEMPLATE( {}, {}, =default, {}, =default, false, false, false, false) + TEST_TEMPLATE( {}, {}, =default, {}, {}, false, false, false, false) + TEST_TEMPLATE( {}, {}, {}, =default, =default, false, false, false, false) + TEST_TEMPLATE( {}, {}, {}, =default, {}, false, false, false, false) + TEST_TEMPLATE( {}, {}, {}, {}, =default, false, false, false, false) + TEST_TEMPLATE( {}, {}, {}, {}, {}, false, false, false, false) +#undef TEST_TEMPLATE + +#define TEST_TEMPLATE(CC, MC, CA, MA) \ + { \ + struct A \ + { \ + A(const A&) CC; \ + A(A&&) MC; \ + A& operator=(const A&) CA; \ + A& operator=(A&&) MA; \ + }; \ + static_assert(!is_trivially_copy_constructible_v>, ""); \ + static_assert(!is_trivially_move_constructible_v>, ""); \ + static_assert(!is_trivially_copy_assignable_v>, ""); \ + static_assert(!is_trivially_move_assignable_v>, ""); \ + } + TEST_TEMPLATE(=default, =default, =default, =default) + TEST_TEMPLATE(=default, =default, =default, {}) + TEST_TEMPLATE(=default, =default, {}, =default) + TEST_TEMPLATE(=default, =default, {}, {}) + TEST_TEMPLATE(=default, {}, =default, =default) + TEST_TEMPLATE(=default, {}, =default, {}) + TEST_TEMPLATE(=default, {}, {}, =default) + TEST_TEMPLATE(=default, {}, {}, {}) + TEST_TEMPLATE( {}, =default, =default, =default) + TEST_TEMPLATE( {}, =default, =default, {}) + TEST_TEMPLATE( {}, =default, {}, =default) + TEST_TEMPLATE( {}, =default, {}, {}) + TEST_TEMPLATE( {}, {}, =default, =default) + TEST_TEMPLATE( {}, {}, =default, {}) + TEST_TEMPLATE( {}, {}, {}, =default) + TEST_TEMPLATE( {}, {}, {}, {}) +#undef TEST_TEMPLATE + + static_assert(is_trivially_copy_constructible_v>, ""); + static_assert(is_trivially_move_constructible_v>, ""); + static_assert(is_trivially_copy_assignable_v>, ""); + static_assert(is_trivially_move_assignable_v>, ""); +} -- 2.30.2