* [Patch] Forward triviality in variant
@ 2017-05-30 6:36 Tim Shen via gcc-patches
2017-05-30 9:41 ` Tim Shen via gcc-patches
0 siblings, 1 reply; 11+ messages in thread
From: Tim Shen via gcc-patches @ 2017-05-30 6:36 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
This patch implements
<https://lichray.github.io/trivially_variant.html>, but with more
changes than the proposal's. It
1) Creates __detail::__variant::_Traits as a centralized place to hold
common (but not all yet) compile-time conditions.
2) Changes the noexcept conditions for the (copy|move) (ctor|assign)
SMFs, so that when one is trivial, one is also noexcept. It's not the
same as p0088r3, nor p0088r3 + D0602R1 anymore.
3) Creates 4 structs, namely (_Copy|_Move)_(ctor|assign)_(base|alias)
for dispatch on triviality. The code that were originally in
_Variant_base are moved into these four structs. There aren't
functional changes except for more triviality.
Sorry for having a large patch. Do tell me if you want me to split it.
Tested on x86_64-linux-gnu.
Thanks!
--
Regards,
Tim Shen
[-- Attachment #2: a.diff --]
[-- Type: text/x-patch, Size: 23828 bytes --]
commit a4db7d21c6e4223300861114931eb0ef78bef1a6
Author: Tim Shen <timshen@google.com>
Date: Mon May 29 22:44:42 2017 -0700
2017-05-30 Tim Shen <timshen@google.com>
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.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5182c..8736fcc75bc 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -290,6 +290,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ref_cast<_Tp>(__t));
}
+ template<typename... _Types>
+ struct _Traits
+ {
+ static constexpr bool is_default_constructible_v =
+ is_default_constructible_v<typename _Nth_type<0, _Types...>::type>;
+ static constexpr bool is_copy_constructible_v =
+ __and_<is_copy_constructible<_Types>...>::value;
+ static constexpr bool is_move_constructible_v =
+ __and_<is_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assignable_v =
+ is_copy_constructible_v && is_move_constructible_v
+ && __and_<is_copy_assignable<_Types>...>::value;
+ static constexpr bool is_move_assignable_v =
+ is_move_constructible_v
+ && __and_<is_move_assignable<_Types>...>::value;
+
+ static constexpr bool is_copy_ctor_trivial =
+ __and_<is_trivially_copy_constructible<_Types>...>::value;
+ static constexpr bool is_move_ctor_trivial =
+ __and_<is_trivially_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assign_trivial =
+ __and_<is_trivially_copy_assignable<_Types>...>::value;
+ static constexpr bool is_move_assign_trivial =
+ __and_<is_trivially_move_assignable<_Types>...>::value;
+ static constexpr bool is_dtor_trivial =
+ __and_<is_trivially_destructible<_Types>...>::value;
+
+ static constexpr bool is_default_ctor_noexcept =
+ is_nothrow_default_constructible_v<
+ typename _Nth_type<0, _Types...>::type>;
+ static constexpr bool is_copy_ctor_noexcept =
+ is_copy_ctor_trivial;
+ static constexpr bool is_move_ctor_noexcept =
+ is_move_ctor_trivial
+ || __and_<is_nothrow_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assign_noexcept =
+ is_copy_assign_trivial;
+ static constexpr bool is_move_assign_noexcept =
+ is_move_assign_trivial ||
+ (is_move_ctor_noexcept
+ && __and_<is_nothrow_move_assignable<_Types>...>::value);
+ };
+
// Defines members and ctors.
template<typename... _Types>
union _Variadic_union { };
@@ -355,6 +398,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Variant_storage()
{ _M_reset(); }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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;
@@ -374,59 +430,114 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void _M_reset()
{ _M_index = variant_npos; }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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<typename... _Types>
- 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...>::is_dtor_trivial, _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.
- _Variant_base(const _Variant_base& __rhs)
+ template<bool, typename... _Types>
+ struct _Copy_ctor_base : _Variant_storage_alias<_Types...>
+ {
+ using _Base = _Variant_storage_alias<_Types...>;
+ using _Base::_Base;
+
+ _Copy_ctor_base(const _Copy_ctor_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept)
{
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&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Copy_ctor_base& operator=(const _Copy_ctor_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Copy_ctor_base& operator=(_Copy_ctor_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Copy_ctor_base<true, _Types...> : _Variant_storage_alias<_Types...>
+ {
+ using _Base = _Variant_storage_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_ctor_alias =
+ _Copy_ctor_base<_Traits<_Types...>::is_copy_ctor_trivial, _Types...>;
+
+ template<bool, typename... _Types>
+ 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...>::is_move_ctor_noexcept)
{
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<size_t _Np, typename... _Args>
- 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& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Move_ctor_base& operator=(const _Move_ctor_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Move_ctor_base& operator=(_Move_ctor_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Move_ctor_base<true, _Types...> : _Copy_ctor_alias<_Types...>
+ {
+ using _Base = _Copy_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_ctor_alias =
+ _Move_ctor_base<_Traits<_Types...>::is_move_ctor_trivial, _Types...>;
+
+ template<bool, typename... _Types>
+ 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...>::is_copy_assign_noexcept)
{
if (this->_M_index == __rhs._M_index)
{
@@ -434,16 +545,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 (...)
{
@@ -455,12 +566,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return *this;
}
- void _M_destructive_move(_Variant_base&& __rhs)
+ _Copy_assign_base(const _Copy_assign_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Copy_assign_base(_Copy_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Copy_assign_base& operator=(_Copy_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Copy_assign_base<true, _Types...> : _Move_ctor_alias<_Types...>
+ {
+ using _Base = _Move_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_assign_alias =
+ _Copy_assign_base<_Traits<_Types...>::is_copy_assign_trivial,
+ _Types...>;
+
+ template<bool, typename... _Types>
+ 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 (...)
{
@@ -469,40 +606,81 @@ _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...>::is_move_assign_noexcept)
{
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<void*>(static_cast<const void*>(
- std::addressof(_Storage::_M_u)));
- }
+ _Move_assign_base(const _Move_assign_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Move_assign_base(_Move_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Move_assign_base& operator=(const _Move_assign_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ };
- constexpr bool
- _M_valid() const noexcept
- {
- return this->_M_index !=
- typename _Storage::__index_type(variant_npos);
- }
+ template<typename... _Types>
+ struct _Move_assign_base<true, _Types...> : _Copy_assign_alias<_Types...>
+ {
+ using _Base = _Copy_assign_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_assign_alias =
+ _Move_assign_base<_Traits<_Types...>::is_move_assign_trivial,
+ _Types...>;
+
+ template<typename... _Types>
+ struct _Variant_base : _Move_assign_alias<_Types...>
+ {
+ using _Base = _Move_assign_alias<_Types...>;
+
+ constexpr
+ _Variant_base()
+ noexcept(_Traits<_Types...>::is_default_ctor_noexcept)
+ : _Variant_base(in_place_index<0>) { }
+
+ template<size_t _Np, typename... _Args>
+ constexpr explicit
+ _Variant_base(in_place_index_t<_Np> __i, _Args&&... __args)
+ : _Base(__i, std::forward<_Args>(__args)...)
+ { }
+
+ _Variant_base(const _Variant_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Variant_base(_Variant_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Variant_base& operator=(const _Variant_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Variant_base& operator=(_Variant_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
};
// For how many times does _Tp appear in _Tuple?
@@ -877,16 +1055,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...>::is_default_constructible_v,
+ 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...>::is_copy_constructible_v,
+ __detail::__variant::_Traits<_Types...>::is_copy_assignable_v,
+ __detail::__variant::_Traits<_Types...>::is_move_constructible_v,
+ __detail::__variant::_Traits<_Types...>::is_move_assignable_v,
variant<_Types...>>
{
private:
@@ -899,9 +1074,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...>::is_default_constructible_v,
+ variant<_Types...>>;
template<typename _Tp>
static constexpr bool
@@ -928,12 +1103,19 @@ _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() noexcept(_Traits::is_default_ctor_noexcept) = default;
+ variant(const variant& __rhs)
+ noexcept(_Traits::is_copy_ctor_noexcept) = default;
variant(variant&&)
- noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = default;
+ noexcept(_Traits::is_move_ctor_noexcept) = default;
+ variant& operator=(const variant&)
+ noexcept(_Traits::is_copy_assign_noexcept) = default;
+ variant& operator=(variant&&)
+ noexcept(_Traits::is_move_assign_noexcept) = default;
+ ~variant() = default;
template<typename _Tp,
typename = enable_if_t<!is_same_v<decay_t<_Tp>, variant>>,
@@ -942,7 +1124,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<typename _Tp, typename... _Args,
@@ -950,7 +1133,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& is_constructible_v<_Tp, _Args&&...>>>
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<typename _Tp, typename _Up, typename... _Args,
@@ -983,13 +1167,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
- ~variant() = default;
-
- variant& operator=(const variant&) = default;
- variant& operator=(variant&&)
- noexcept((is_nothrow_move_constructible_v<_Types> && ...)
- && (is_nothrow_move_assignable_v<_Types> && ...)) = default;
-
template<typename _Tp>
enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
&& is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
@@ -1084,7 +1261,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..dbf5cffac57 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<variant<int, string>>, "");
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>>, "");
{
variant<int> a;
- static_assert(!noexcept(variant<int>(a)), "");
+ static_assert(noexcept(variant<int>(a)), "");
}
{
variant<string> a;
@@ -103,7 +105,7 @@ void copy_ctor()
}
{
variant<int, char> a;
- static_assert(!noexcept(variant<int, char>(a)), "");
+ static_assert(noexcept(variant<int, char>(a)), "");
}
}
@@ -111,6 +113,8 @@ void move_ctor()
{
static_assert(is_move_constructible_v<variant<int, string>>, "");
static_assert(!is_move_constructible_v<variant<AllDeleted, string>>, "");
+ 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>>())), "");
static_assert(noexcept(variant<int, DefaultNoexcept>(declval<variant<int, DefaultNoexcept>>())), "");
}
@@ -148,13 +152,15 @@ void copy_assign()
{
static_assert(is_copy_assignable_v<variant<int, string>>, "");
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>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = a), "");
}
{
variant<DefaultNoexcept> a;
- static_assert(!noexcept(a = a), "");
+ static_assert(noexcept(a = a), "");
}
}
@@ -162,6 +168,8 @@ void move_assign()
{
static_assert(is_move_assignable_v<variant<int, string>>, "");
static_assert(!is_move_assignable_v<variant<AllDeleted, string>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<int>>, "");
+ static_assert(!is_trivially_move_assignable_v<variant<string>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = std::move(a)), "");
@@ -454,3 +462,75 @@ void test_emplace()
static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0), "");
static_assert(!has_index_emplace<variant<AllDeleted>, 0>(0), "");
}
+
+void test_triviality()
+{
+#define TEST_TEMPLATE(CC, MC, CA, MA, CC_VAL, MC_VAL, CA_VAL, MA_VAL) \
+ { \
+ struct A \
+ { \
+ 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<variant<A>>, ""); \
+ static_assert(MC_VAL is_trivially_move_constructible_v<variant<A>>, ""); \
+ static_assert(CA_VAL is_trivially_copy_assignable_v<variant<A>>, ""); \
+ static_assert(MA_VAL is_trivially_move_assignable_v<variant<A>>, ""); \
+ }
+ 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
+
+#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<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_constructible_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_copy_assignable_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_assignable_v<variant<AllDeleted, A>>, ""); \
+ }
+ 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<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_constructible_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_copy_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-05-30 6:36 [Patch] Forward triviality in variant Tim Shen via gcc-patches
@ 2017-05-30 9:41 ` Tim Shen via gcc-patches
2017-06-01 15:13 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Tim Shen via gcc-patches @ 2017-05-30 9:41 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 289 bytes --]
On Mon, May 29, 2017 at 11:29 PM, Tim Shen <timshen@google.com> wrote:
> This patch implements
> <https://lichray.github.io/trivially_variant.html>, but with more
Actually, it didn't. The copy assign and move assign conditions are
wrong in the patch. Fixed those.
--
Regards,
Tim Shen
[-- Attachment #2: b.diff --]
[-- Type: text/x-patch, Size: 26360 bytes --]
commit 03387ef5007e171e4aeceeddf4d856caffeedb41
Author: Tim Shen <timshen@google.com>
Date: Mon May 29 22:44:42 2017 -0700
2017-05-30 Tim Shen <timshen@google.com>
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.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5182c..f81b815af09 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ref_cast<_Tp>(__t));
}
+ template<typename... _Types>
+ struct _Traits
+ {
+ static constexpr bool is_default_constructible_v =
+ is_default_constructible_v<typename _Nth_type<0, _Types...>::type>;
+ static constexpr bool is_copy_constructible_v =
+ __and_<is_copy_constructible<_Types>...>::value;
+ static constexpr bool is_move_constructible_v =
+ __and_<is_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assignable_v =
+ is_copy_constructible_v && is_move_constructible_v
+ && __and_<is_copy_assignable<_Types>...>::value;
+ static constexpr bool is_move_assignable_v =
+ is_move_constructible_v
+ && __and_<is_move_assignable<_Types>...>::value;
+
+ static constexpr bool is_dtor_trivial =
+ __and_<is_trivially_destructible<_Types>...>::value;
+ static constexpr bool is_copy_ctor_trivial =
+ __and_<is_trivially_copy_constructible<_Types>...>::value;
+ static constexpr bool is_move_ctor_trivial =
+ __and_<is_trivially_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assign_trivial =
+ is_dtor_trivial
+ && is_copy_ctor_trivial
+ && __and_<is_trivially_copy_assignable<_Types>...>::value;
+ static constexpr bool is_move_assign_trivial =
+ is_dtor_trivial
+ && is_move_ctor_trivial
+ && __and_<is_trivially_move_assignable<_Types>...>::value;
+
+ static constexpr bool is_default_ctor_noexcept =
+ is_nothrow_default_constructible_v<
+ typename _Nth_type<0, _Types...>::type>;
+ static constexpr bool is_copy_ctor_noexcept =
+ is_copy_ctor_trivial;
+ static constexpr bool is_move_ctor_noexcept =
+ is_move_ctor_trivial
+ || __and_<is_nothrow_move_constructible<_Types>...>::value;
+ static constexpr bool is_copy_assign_noexcept =
+ is_copy_assign_trivial;
+ static constexpr bool is_move_assign_noexcept =
+ is_move_assign_trivial ||
+ (is_move_ctor_noexcept
+ && __and_<is_nothrow_move_assignable<_Types>...>::value);
+ };
+
// Defines members and ctors.
template<typename... _Types>
union _Variadic_union { };
@@ -355,6 +402,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Variant_storage()
{ _M_reset(); }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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;
@@ -374,59 +434,114 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void _M_reset()
{ _M_index = variant_npos; }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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<typename... _Types>
- 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...>::is_dtor_trivial, _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.
- _Variant_base(const _Variant_base& __rhs)
+ template<bool, typename... _Types>
+ struct _Copy_ctor_base : _Variant_storage_alias<_Types...>
+ {
+ using _Base = _Variant_storage_alias<_Types...>;
+ using _Base::_Base;
+
+ _Copy_ctor_base(const _Copy_ctor_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept)
{
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&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Copy_ctor_base& operator=(const _Copy_ctor_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Copy_ctor_base& operator=(_Copy_ctor_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Copy_ctor_base<true, _Types...> : _Variant_storage_alias<_Types...>
+ {
+ using _Base = _Variant_storage_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_ctor_alias =
+ _Copy_ctor_base<_Traits<_Types...>::is_copy_ctor_trivial, _Types...>;
+
+ template<bool, typename... _Types>
+ 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...>::is_move_ctor_noexcept)
{
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<size_t _Np, typename... _Args>
- 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& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Move_ctor_base& operator=(const _Move_ctor_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Move_ctor_base& operator=(_Move_ctor_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Move_ctor_base<true, _Types...> : _Copy_ctor_alias<_Types...>
+ {
+ using _Base = _Copy_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_ctor_alias =
+ _Move_ctor_base<_Traits<_Types...>::is_move_ctor_trivial, _Types...>;
+
+ template<bool, typename... _Types>
+ 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...>::is_copy_assign_noexcept)
{
if (this->_M_index == __rhs._M_index)
{
@@ -434,16 +549,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 (...)
{
@@ -455,12 +570,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return *this;
}
- void _M_destructive_move(_Variant_base&& __rhs)
+ _Copy_assign_base(const _Copy_assign_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Copy_assign_base(_Copy_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Copy_assign_base& operator=(_Copy_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
+ };
+
+ template<typename... _Types>
+ struct _Copy_assign_base<true, _Types...> : _Move_ctor_alias<_Types...>
+ {
+ using _Base = _Move_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_assign_alias =
+ _Copy_assign_base<_Traits<_Types...>::is_copy_assign_trivial,
+ _Types...>;
+
+ template<bool, typename... _Types>
+ 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 (...)
{
@@ -469,40 +610,81 @@ _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...>::is_move_assign_noexcept)
{
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<void*>(static_cast<const void*>(
- std::addressof(_Storage::_M_u)));
- }
+ _Move_assign_base(const _Move_assign_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Move_assign_base(_Move_assign_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Move_assign_base& operator=(const _Move_assign_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ };
- constexpr bool
- _M_valid() const noexcept
- {
- return this->_M_index !=
- typename _Storage::__index_type(variant_npos);
- }
+ template<typename... _Types>
+ struct _Move_assign_base<true, _Types...> : _Copy_assign_alias<_Types...>
+ {
+ using _Base = _Copy_assign_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_assign_alias =
+ _Move_assign_base<_Traits<_Types...>::is_move_assign_trivial,
+ _Types...>;
+
+ template<typename... _Types>
+ struct _Variant_base : _Move_assign_alias<_Types...>
+ {
+ using _Base = _Move_assign_alias<_Types...>;
+
+ constexpr
+ _Variant_base()
+ noexcept(_Traits<_Types...>::is_default_ctor_noexcept)
+ : _Variant_base(in_place_index<0>) { }
+
+ template<size_t _Np, typename... _Args>
+ constexpr explicit
+ _Variant_base(in_place_index_t<_Np> __i, _Args&&... __args)
+ : _Base(__i, std::forward<_Args>(__args)...)
+ { }
+
+ _Variant_base(const _Variant_base& __rhs)
+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
+ _Variant_base(_Variant_base&&)
+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
+ _Variant_base& operator=(const _Variant_base&)
+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
+ _Variant_base& operator=(_Variant_base&&)
+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
};
// For how many times does _Tp appear in _Tuple?
@@ -877,16 +1059,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...>::is_default_constructible_v,
+ 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...>::is_copy_constructible_v,
+ __detail::__variant::_Traits<_Types...>::is_copy_assignable_v,
+ __detail::__variant::_Traits<_Types...>::is_move_constructible_v,
+ __detail::__variant::_Traits<_Types...>::is_move_assignable_v,
variant<_Types...>>
{
private:
@@ -899,9 +1078,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...>::is_default_constructible_v,
+ variant<_Types...>>;
template<typename _Tp>
static constexpr bool
@@ -928,12 +1107,19 @@ _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() noexcept(_Traits::is_default_ctor_noexcept) = default;
+ variant(const variant& __rhs)
+ noexcept(_Traits::is_copy_ctor_noexcept) = default;
variant(variant&&)
- noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = default;
+ noexcept(_Traits::is_move_ctor_noexcept) = default;
+ variant& operator=(const variant&)
+ noexcept(_Traits::is_copy_assign_noexcept) = default;
+ variant& operator=(variant&&)
+ noexcept(_Traits::is_move_assign_noexcept) = default;
+ ~variant() = default;
template<typename _Tp,
typename = enable_if_t<!is_same_v<decay_t<_Tp>, variant>>,
@@ -942,7 +1128,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<typename _Tp, typename... _Args,
@@ -950,7 +1137,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& is_constructible_v<_Tp, _Args&&...>>>
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<typename _Tp, typename _Up, typename... _Args,
@@ -983,13 +1171,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
- ~variant() = default;
-
- variant& operator=(const variant&) = default;
- variant& operator=(variant&&)
- noexcept((is_nothrow_move_constructible_v<_Types> && ...)
- && (is_nothrow_move_assignable_v<_Types> && ...)) = default;
-
template<typename _Tp>
enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
&& is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
@@ -1084,7 +1265,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..2e8fc341d4d 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<variant<int, string>>, "");
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>>, "");
{
variant<int> a;
- static_assert(!noexcept(variant<int>(a)), "");
+ static_assert(noexcept(variant<int>(a)), "");
}
{
variant<string> a;
@@ -103,7 +105,7 @@ void copy_ctor()
}
{
variant<int, char> a;
- static_assert(!noexcept(variant<int, char>(a)), "");
+ static_assert(noexcept(variant<int, char>(a)), "");
}
}
@@ -111,6 +113,8 @@ void move_ctor()
{
static_assert(is_move_constructible_v<variant<int, string>>, "");
static_assert(!is_move_constructible_v<variant<AllDeleted, string>>, "");
+ 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>>())), "");
static_assert(noexcept(variant<int, DefaultNoexcept>(declval<variant<int, DefaultNoexcept>>())), "");
}
@@ -148,13 +152,15 @@ void copy_assign()
{
static_assert(is_copy_assignable_v<variant<int, string>>, "");
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>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = a), "");
}
{
variant<DefaultNoexcept> a;
- static_assert(!noexcept(a = a), "");
+ static_assert(noexcept(a = a), "");
}
}
@@ -162,6 +168,8 @@ void move_assign()
{
static_assert(is_move_assignable_v<variant<int, string>>, "");
static_assert(!is_move_assignable_v<variant<AllDeleted, string>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<int>>, "");
+ static_assert(!is_trivially_move_assignable_v<variant<string>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = std::move(a)), "");
@@ -454,3 +462,92 @@ void test_emplace()
static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0), "");
static_assert(!has_index_emplace<variant<AllDeleted>, 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<variant<A>>, ""); \
+ static_assert(MC_VAL == is_trivially_move_constructible_v<variant<A>>, ""); \
+ static_assert(CA_VAL == is_trivially_copy_assignable_v<variant<A>>, ""); \
+ static_assert(MA_VAL == is_trivially_move_assignable_v<variant<A>>, ""); \
+ }
+ 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, false)
+ TEST_TEMPLATE(=default, =default, {}, =default, {}, true, false, true, false)
+ TEST_TEMPLATE(=default, =default, {}, {}, =default, true, false, false, false)
+ TEST_TEMPLATE(=default, =default, {}, {}, {}, true, false, false, false)
+ TEST_TEMPLATE(=default, {}, =default, =default, =default, false, true, false, true)
+ TEST_TEMPLATE(=default, {}, =default, =default, {}, false, true, false, 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, 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, =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<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_constructible_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_copy_assignable_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_assignable_v<variant<AllDeleted, A>>, ""); \
+ }
+ 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<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_constructible_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_copy_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-05-30 9:41 ` Tim Shen via gcc-patches
@ 2017-06-01 15:13 ` Jonathan Wakely
2017-06-01 15:21 ` Ville Voutilainen
2017-06-18 19:37 ` Tim Shen via gcc-patches
0 siblings, 2 replies; 11+ messages in thread
From: Jonathan Wakely @ 2017-06-01 15:13 UTC (permalink / raw)
To: Tim Shen; +Cc: libstdc++, gcc-patches
On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index b9824a5182c..f81b815af09 100644
>--- a/libstdc++-v3/include/std/variant
>+++ b/libstdc++-v3/include/std/variant
>@@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __ref_cast<_Tp>(__t));
> }
>
>+ template<typename... _Types>
>+ struct _Traits
>+ {
>+ static constexpr bool is_default_constructible_v =
>+ is_default_constructible_v<typename _Nth_type<0, _Types...>::type>;
>+ static constexpr bool is_copy_constructible_v =
>+ __and_<is_copy_constructible<_Types>...>::value;
>+ static constexpr bool is_move_constructible_v =
>+ __and_<is_move_constructible<_Types>...>::value;
>+ static constexpr bool is_copy_assignable_v =
>+ is_copy_constructible_v && is_move_constructible_v
>+ && __and_<is_copy_assignable<_Types>...>::value;
>+ static constexpr bool is_move_assignable_v =
>+ is_move_constructible_v
>+ && __and_<is_move_assignable<_Types>...>::value;
It seems strange to me that these ones end with _v but the following
ones don't. Could we make them all have no _v suffix?
>+ static constexpr bool is_dtor_trivial =
>+ __and_<is_trivially_destructible<_Types>...>::value;
>+ static constexpr bool is_copy_ctor_trivial =
>+ __and_<is_trivially_copy_constructible<_Types>...>::value;
>+ static constexpr bool is_move_ctor_trivial =
>+ __and_<is_trivially_move_constructible<_Types>...>::value;
>+ static constexpr bool is_copy_assign_trivial =
>+ is_dtor_trivial
>+ && is_copy_ctor_trivial
>+ && __and_<is_trivially_copy_assignable<_Types>...>::value;
>+ static constexpr bool is_move_assign_trivial =
>+ is_dtor_trivial
>+ && is_move_ctor_trivial
>+ && __and_<is_trivially_move_assignable<_Types>...>::value;
>+
>+ static constexpr bool is_default_ctor_noexcept =
>+ is_nothrow_default_constructible_v<
>+ typename _Nth_type<0, _Types...>::type>;
>+ static constexpr bool is_copy_ctor_noexcept =
>+ is_copy_ctor_trivial;
>+ static constexpr bool is_move_ctor_noexcept =
>+ is_move_ctor_trivial
>+ || __and_<is_nothrow_move_constructible<_Types>...>::value;
>+ static constexpr bool is_copy_assign_noexcept =
>+ is_copy_assign_trivial;
>+ static constexpr bool is_move_assign_noexcept =
>+ is_move_assign_trivial ||
>+ (is_move_ctor_noexcept
>+ && __and_<is_nothrow_move_assignable<_Types>...>::value);
>+ };
Does using __and_ for any of those traits reduce the limit on the
number of alternatives in a variant? We switched to using fold
expressions in some contexts to avoid very deep instantiations, but I
don't know if these will hit the same problem, but it looks like it
will.
> // Defines members and ctors.
> template<typename... _Types>
> union _Variadic_union { };
>@@ -355,6 +402,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ~_Variant_storage()
> { _M_reset(); }
>
>+ void*
>+ _M_storage() const
>+ {
>+ return const_cast<void*>(static_cast<const void*>(
>+ 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;
>@@ -374,59 +434,114 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> void _M_reset()
> { _M_index = variant_npos; }
>
>+ void*
>+ _M_storage() const
>+ {
>+ return const_cast<void*>(static_cast<const void*>(
>+ 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<typename... _Types>
>- 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...>::is_dtor_trivial, _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.
>
>- _Variant_base(const _Variant_base& __rhs)
>+ template<bool, typename... _Types>
>+ struct _Copy_ctor_base : _Variant_storage_alias<_Types...>
>+ {
>+ using _Base = _Variant_storage_alias<_Types...>;
>+ using _Base::_Base;
>+
>+ _Copy_ctor_base(const _Copy_ctor_base& __rhs)
>+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept)
> {
> 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&&)
>+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
>+ _Copy_ctor_base& operator=(const _Copy_ctor_base&)
>+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
>+ _Copy_ctor_base& operator=(_Copy_ctor_base&&)
>+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
>+ };
>+
>+ template<typename... _Types>
>+ struct _Copy_ctor_base<true, _Types...> : _Variant_storage_alias<_Types...>
>+ {
>+ using _Base = _Variant_storage_alias<_Types...>;
>+ using _Base::_Base;
>+ };
>+
>+ template<typename... _Types>
>+ using _Copy_ctor_alias =
>+ _Copy_ctor_base<_Traits<_Types...>::is_copy_ctor_trivial, _Types...>;
>+
>+ template<bool, typename... _Types>
>+ 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...>::is_move_ctor_noexcept)
> {
> 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<size_t _Np, typename... _Args>
>- 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& __rhs)
>+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
>+ _Move_ctor_base& operator=(const _Move_ctor_base&)
>+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
>+ _Move_ctor_base& operator=(_Move_ctor_base&&)
>+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
>+ };
>+
>+ template<typename... _Types>
>+ struct _Move_ctor_base<true, _Types...> : _Copy_ctor_alias<_Types...>
>+ {
>+ using _Base = _Copy_ctor_alias<_Types...>;
>+ using _Base::_Base;
>+ };
>+
>+ template<typename... _Types>
>+ using _Move_ctor_alias =
>+ _Move_ctor_base<_Traits<_Types...>::is_move_ctor_trivial, _Types...>;
>+
>+ template<bool, typename... _Types>
>+ 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...>::is_copy_assign_noexcept)
> {
> if (this->_M_index == __rhs._M_index)
> {
>@@ -434,16 +549,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 (...)
> {
>@@ -455,12 +570,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return *this;
> }
>
>- void _M_destructive_move(_Variant_base&& __rhs)
>+ _Copy_assign_base(const _Copy_assign_base& __rhs)
>+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
>+ _Copy_assign_base(_Copy_assign_base&&)
>+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
>+ _Copy_assign_base& operator=(_Copy_assign_base&&)
>+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
>+ };
>+
>+ template<typename... _Types>
>+ struct _Copy_assign_base<true, _Types...> : _Move_ctor_alias<_Types...>
>+ {
>+ using _Base = _Move_ctor_alias<_Types...>;
>+ using _Base::_Base;
>+ };
>+
>+ template<typename... _Types>
>+ using _Copy_assign_alias =
>+ _Copy_assign_base<_Traits<_Types...>::is_copy_assign_trivial,
>+ _Types...>;
>+
>+ template<bool, typename... _Types>
>+ 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 (...)
> {
>@@ -469,40 +610,81 @@ _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...>::is_move_assign_noexcept)
> {
> 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<void*>(static_cast<const void*>(
>- std::addressof(_Storage::_M_u)));
>- }
>+ _Move_assign_base(const _Move_assign_base& __rhs)
>+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
>+ _Move_assign_base(_Move_assign_base&&)
>+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
>+ _Move_assign_base& operator=(const _Move_assign_base&)
>+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
>+ };
>
>- constexpr bool
>- _M_valid() const noexcept
>- {
>- return this->_M_index !=
>- typename _Storage::__index_type(variant_npos);
>- }
>+ template<typename... _Types>
>+ struct _Move_assign_base<true, _Types...> : _Copy_assign_alias<_Types...>
>+ {
>+ using _Base = _Copy_assign_alias<_Types...>;
>+ using _Base::_Base;
>+ };
>+
>+ template<typename... _Types>
>+ using _Move_assign_alias =
>+ _Move_assign_base<_Traits<_Types...>::is_move_assign_trivial,
>+ _Types...>;
>+
>+ template<typename... _Types>
>+ struct _Variant_base : _Move_assign_alias<_Types...>
>+ {
>+ using _Base = _Move_assign_alias<_Types...>;
>+
>+ constexpr
>+ _Variant_base()
>+ noexcept(_Traits<_Types...>::is_default_ctor_noexcept)
>+ : _Variant_base(in_place_index<0>) { }
>+
>+ template<size_t _Np, typename... _Args>
>+ constexpr explicit
>+ _Variant_base(in_place_index_t<_Np> __i, _Args&&... __args)
>+ : _Base(__i, std::forward<_Args>(__args)...)
>+ { }
>+
>+ _Variant_base(const _Variant_base& __rhs)
>+ noexcept(_Traits<_Types...>::is_copy_ctor_noexcept) = default;
>+ _Variant_base(_Variant_base&&)
>+ noexcept(_Traits<_Types...>::is_move_ctor_noexcept) = default;
>+ _Variant_base& operator=(const _Variant_base&)
>+ noexcept(_Traits<_Types...>::is_copy_assign_noexcept) = default;
>+ _Variant_base& operator=(_Variant_base&&)
>+ noexcept(_Traits<_Types...>::is_move_assign_noexcept) = default;
> };
>
> // For how many times does _Tp appear in _Tuple?
>@@ -877,16 +1059,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...>::is_default_constructible_v,
>+ 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...>::is_copy_constructible_v,
>+ __detail::__variant::_Traits<_Types...>::is_copy_assignable_v,
>+ __detail::__variant::_Traits<_Types...>::is_move_constructible_v,
>+ __detail::__variant::_Traits<_Types...>::is_move_assignable_v,
> variant<_Types...>>
> {
> private:
>@@ -899,9 +1078,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...>::is_default_constructible_v,
>+ variant<_Types...>>;
>
> template<typename _Tp>
> static constexpr bool
>@@ -928,12 +1107,19 @@ _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() noexcept(_Traits::is_default_ctor_noexcept) = default;
Do we need the exception specifications here? Will the =default make
the right thing happen anyway? (And if not, won't we get an error by
trying to define the constructors as noexcept when the implicit
definition would not be noexcept?)
>+ variant(const variant& __rhs)
>+ noexcept(_Traits::is_copy_ctor_noexcept) = default;
> variant(variant&&)
>- noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = default;
>+ noexcept(_Traits::is_move_ctor_noexcept) = default;
>+ variant& operator=(const variant&)
>+ noexcept(_Traits::is_copy_assign_noexcept) = default;
>+ variant& operator=(variant&&)
>+ noexcept(_Traits::is_move_assign_noexcept) = default;
>+ ~variant() = default;
>
> template<typename _Tp,
> typename = enable_if_t<!is_same_v<decay_t<_Tp>, variant>>,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 15:13 ` Jonathan Wakely
@ 2017-06-01 15:21 ` Ville Voutilainen
2017-06-01 15:29 ` Jonathan Wakely
2017-06-18 19:37 ` Tim Shen via gcc-patches
1 sibling, 1 reply; 11+ messages in thread
From: Ville Voutilainen @ 2017-06-01 15:21 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Tim Shen, libstdc++, gcc-patches
On 1 June 2017 at 18:13, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>>
>> diff --git a/libstdc++-v3/include/std/variant
>> b/libstdc++-v3/include/std/variant
>> index b9824a5182c..f81b815af09 100644
>> --- a/libstdc++-v3/include/std/variant
>> +++ b/libstdc++-v3/include/std/variant
>> @@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> __ref_cast<_Tp>(__t));
>> }
>>
>> + template<typename... _Types>
>> + struct _Traits
>> + {
>> + static constexpr bool is_default_constructible_v =
>> + is_default_constructible_v<typename _Nth_type<0,
>> _Types...>::type>;
>> + static constexpr bool is_copy_constructible_v =
>> + __and_<is_copy_constructible<_Types>...>::value;
>> + static constexpr bool is_move_constructible_v =
>> + __and_<is_move_constructible<_Types>...>::value;
>> + static constexpr bool is_copy_assignable_v =
>> + is_copy_constructible_v && is_move_constructible_v
>> + && __and_<is_copy_assignable<_Types>...>::value;
>> + static constexpr bool is_move_assignable_v =
>> + is_move_constructible_v
>> + && __and_<is_move_assignable<_Types>...>::value;
>
>
> It seems strange to me that these ones end with _v but the following
> ones don't. Could we make them all have no _v suffix?
Seems to me worth considering to rather make all of them have a _v suffix. :)
>
>> + static constexpr bool is_dtor_trivial =
>> + __and_<is_trivially_destructible<_Types>...>::value;
They all seem to be shortcuts for something::value, so it seems to me
logical to have
them all be _v.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 15:21 ` Ville Voutilainen
@ 2017-06-01 15:29 ` Jonathan Wakely
2017-06-01 15:43 ` Ville Voutilainen
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2017-06-01 15:29 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: Tim Shen, libstdc++, gcc-patches
On 01/06/17 18:21 +0300, Ville Voutilainen wrote:
>On 1 June 2017 at 18:13, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>>>
>>> diff --git a/libstdc++-v3/include/std/variant
>>> b/libstdc++-v3/include/std/variant
>>> index b9824a5182c..f81b815af09 100644
>>> --- a/libstdc++-v3/include/std/variant
>>> +++ b/libstdc++-v3/include/std/variant
>>> @@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>> __ref_cast<_Tp>(__t));
>>> }
>>>
>>> + template<typename... _Types>
>>> + struct _Traits
>>> + {
>>> + static constexpr bool is_default_constructible_v =
>>> + is_default_constructible_v<typename _Nth_type<0,
>>> _Types...>::type>;
>>> + static constexpr bool is_copy_constructible_v =
>>> + __and_<is_copy_constructible<_Types>...>::value;
>>> + static constexpr bool is_move_constructible_v =
>>> + __and_<is_move_constructible<_Types>...>::value;
>>> + static constexpr bool is_copy_assignable_v =
>>> + is_copy_constructible_v && is_move_constructible_v
>>> + && __and_<is_copy_assignable<_Types>...>::value;
>>> + static constexpr bool is_move_assignable_v =
>>> + is_move_constructible_v
>>> + && __and_<is_move_assignable<_Types>...>::value;
>>
>>
>> It seems strange to me that these ones end with _v but the following
>> ones don't. Could we make them all have no _v suffix?
>
>Seems to me worth considering to rather make all of them have a _v suffix. :)
>>
>>> + static constexpr bool is_dtor_trivial =
>>> + __and_<is_trivially_destructible<_Types>...>::value;
>
>
>They all seem to be shortcuts for something::value, so it seems to me
>logical to have
>them all be _v.
The _v suffixes in the standard are there to distinguish std::foo from
std::foo_v, but we don't have that problem.
__variant::_Traits<T...>::foo is a unique name, we don't need the
suffix, it's just noise.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 15:29 ` Jonathan Wakely
@ 2017-06-01 15:43 ` Ville Voutilainen
2017-06-01 16:03 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Ville Voutilainen @ 2017-06-01 15:43 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Tim Shen, libstdc++, gcc-patches
On 1 June 2017 at 18:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>> They all seem to be shortcuts for something::value, so it seems to me
>> logical to have
>> them all be _v.
>
>
> The _v suffixes in the standard are there to distinguish std::foo from
> std::foo_v, but we don't have that problem.
Wouldn't necessarily hurt to follow the same naming convention idea as
the standard, but sure, we
don't have that problem, agreed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 15:43 ` Ville Voutilainen
@ 2017-06-01 16:03 ` Jonathan Wakely
2017-06-01 16:07 ` Ville Voutilainen
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2017-06-01 16:03 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: Tim Shen, libstdc++, gcc-patches
On 01/06/17 18:43 +0300, Ville Voutilainen wrote:
>On 1 June 2017 at 18:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>>> They all seem to be shortcuts for something::value, so it seems to me
>>> logical to have
>>> them all be _v.
>>
>>
>> The _v suffixes in the standard are there to distinguish std::foo from
>> std::foo_v, but we don't have that problem.
>
>Wouldn't necessarily hurt to follow the same naming convention idea as
>the standard, but sure, we
>don't have that problem, agreed.
It's not consistent in the standard:
- numeric_limits<T>::is_specialized
- std::chrono::system_clock::is_steady
- std::atomic<T>::is_always_lock_free
And that's OK, because it would be a silly rule that said all boolean
constants should end in _v, it would just be noise.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 16:03 ` Jonathan Wakely
@ 2017-06-01 16:07 ` Ville Voutilainen
2017-06-01 16:13 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Ville Voutilainen @ 2017-06-01 16:07 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Tim Shen, libstdc++, gcc-patches
On 1 June 2017 at 19:03, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 01/06/17 18:43 +0300, Ville Voutilainen wrote:
>>
>> On 1 June 2017 at 18:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>
>>>> They all seem to be shortcuts for something::value, so it seems to me
>>>> logical to have
>>>> them all be _v.
>>> The _v suffixes in the standard are there to distinguish std::foo from
>>> std::foo_v, but we don't have that problem.
>> Wouldn't necessarily hurt to follow the same naming convention idea as
>> the standard, but sure, we
>> don't have that problem, agreed.
> It's not consistent in the standard:
> - numeric_limits<T>::is_specialized
> - std::chrono::system_clock::is_steady
> - std::atomic<T>::is_always_lock_free
>
> And that's OK, because it would be a silly rule that said all boolean
> constants should end in _v, it would just be noise.
But I didn't suggest such a rule, merely that if we are doing with a
trait-like variable
that shortcuts a ::value, then we could entertain using _v.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 16:07 ` Ville Voutilainen
@ 2017-06-01 16:13 ` Jonathan Wakely
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Wakely @ 2017-06-01 16:13 UTC (permalink / raw)
To: Ville Voutilainen; +Cc: Tim Shen, libstdc++, gcc-patches
On 01/06/17 19:07 +0300, Ville Voutilainen wrote:
>On 1 June 2017 at 19:03, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 01/06/17 18:43 +0300, Ville Voutilainen wrote:
>>>
>>> On 1 June 2017 at 18:29, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>>
>>>>> They all seem to be shortcuts for something::value, so it seems to me
>>>>> logical to have
>>>>> them all be _v.
>>>> The _v suffixes in the standard are there to distinguish std::foo from
>>>> std::foo_v, but we don't have that problem.
>>> Wouldn't necessarily hurt to follow the same naming convention idea as
>>> the standard, but sure, we
>>> don't have that problem, agreed.
>> It's not consistent in the standard:
>> - numeric_limits<T>::is_specialized
>> - std::chrono::system_clock::is_steady
>> - std::atomic<T>::is_always_lock_free
>>
>> And that's OK, because it would be a silly rule that said all boolean
>> constants should end in _v, it would just be noise.
>
>
>But I didn't suggest such a rule, merely that if we are doing with a
>trait-like variable
>that shortcuts a ::value, then we could entertain using _v.
The trait describes properties of the variant. The fact those
properties are determined by something::value is an implementation
detail, not an important feature that needs to be in the name.
The implementation details should not leak into the public API of the
trait.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-01 15:13 ` Jonathan Wakely
2017-06-01 15:21 ` Ville Voutilainen
@ 2017-06-18 19:37 ` Tim Shen via gcc-patches
2017-06-27 15:43 ` Jonathan Wakely
1 sibling, 1 reply; 11+ messages in thread
From: Tim Shen via gcc-patches @ 2017-06-18 19:37 UTC (permalink / raw)
To: Jonathan Wakely, Ville Voutilainen; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4661 bytes --]
Besides the changes on the comments, I also changed the definition of
_S_trivial_copy_assign and _S_trivial_move_assign to match what union
has. See [class.copy.assign]p9.
On Thu, Jun 1, 2017 at 8:13 AM, Jonathan Wakely wrote:
> On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>>
>> diff --git a/libstdc++-v3/include/std/variant
>> b/libstdc++-v3/include/std/variant
>> index b9824a5182c..f81b815af09 100644
>> --- a/libstdc++-v3/include/std/variant
>> +++ b/libstdc++-v3/include/std/variant
>> @@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> __ref_cast<_Tp>(__t));
>> }
>>
>> + template<typename... _Types>
>> + struct _Traits
>> + {
>> + static constexpr bool is_default_constructible_v =
>> + is_default_constructible_v<typename _Nth_type<0,
>> _Types...>::type>;
>> + static constexpr bool is_copy_constructible_v =
>> + __and_<is_copy_constructible<_Types>...>::value;
>> + static constexpr bool is_move_constructible_v =
>> + __and_<is_move_constructible<_Types>...>::value;
>> + static constexpr bool is_copy_assignable_v =
>> + is_copy_constructible_v && is_move_constructible_v
>> + && __and_<is_copy_assignable<_Types>...>::value;
>> + static constexpr bool is_move_assignable_v =
>> + is_move_constructible_v
>> + && __and_<is_move_assignable<_Types>...>::value;
>
>
> It seems strange to me that these ones end with _v but the following
> ones don't. Could we make them all have no _v suffix?
Done. They are internal traits only for readability, so I shortened
the names and make them libstdc++ style, e.g. _S_copy_ctor.
>
>> + static constexpr bool is_dtor_trivial =
>> + __and_<is_trivially_destructible<_Types>...>::value;
>> + static constexpr bool is_copy_ctor_trivial =
>> + __and_<is_trivially_copy_constructible<_Types>...>::value;
>> + static constexpr bool is_move_ctor_trivial =
>> + __and_<is_trivially_move_constructible<_Types>...>::value;
>> + static constexpr bool is_copy_assign_trivial =
>> + is_dtor_trivial
>> + && is_copy_ctor_trivial
>> + && __and_<is_trivially_copy_assignable<_Types>...>::value;
>> + static constexpr bool is_move_assign_trivial =
>> + is_dtor_trivial
>> + && is_move_ctor_trivial
>> + && __and_<is_trivially_move_assignable<_Types>...>::value;
>> +
>> + static constexpr bool is_default_ctor_noexcept =
>> + is_nothrow_default_constructible_v<
>> + typename _Nth_type<0, _Types...>::type>;
>> + static constexpr bool is_copy_ctor_noexcept =
>> + is_copy_ctor_trivial;
>> + static constexpr bool is_move_ctor_noexcept =
>> + is_move_ctor_trivial
>> + || __and_<is_nothrow_move_constructible<_Types>...>::value;
>> + static constexpr bool is_copy_assign_noexcept =
>> + is_copy_assign_trivial;
>> + static constexpr bool is_move_assign_noexcept =
>> + is_move_assign_trivial ||
>> + (is_move_ctor_noexcept
>> + && __and_<is_nothrow_move_assignable<_Types>...>::value);
>> + };
>
>
> Does using __and_ for any of those traits reduce the limit on the
> number of alternatives in a variant? We switched to using fold
> expressions in some contexts to avoid very deep instantiations, but I
> don't know if these will hit the same problem, but it looks like it
> will.
Done, use fold expression instead. At one point we changed some fold
expressions to __and_, because __and_ has short circuiting; does fold
expressions have short circuits too? Now that I think about it, short
circuiting in a constant fold expression should be a QoI issue.
>> @@ -928,12 +1107,19 @@ _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() noexcept(_Traits::is_default_ctor_noexcept) = default;
>
>
> Do we need the exception specifications here? Will the =default make
> the right thing happen anyway? (And if not, won't we get an error by
> trying to define the constructors as noexcept when the implicit
> definition would not be noexcept?)
Done. Removed unnecessary noexcept qualifiers.
It turns out I mistakenly thought using "variant() = default" means
`variant() noexcept(false) = default`.
--
Regards,
Tim Shen
[-- Attachment #2: c.diff --]
[-- Type: text/x-patch, Size: 24736 bytes --]
commit 919492daf1c5ff78de4a33bdae265ea1acacc446
Author: Tim Shen <timshen@google.com>
Date: Mon May 29 22:44:42 2017 -0700
2017-05-30 Tim Shen <timshen@google.com>
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.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5182c..ef0cba98388 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -290,6 +290,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ref_cast<_Tp>(__t));
}
+ template<typename... _Types>
+ struct _Traits
+ {
+ static constexpr bool _S_default_ctor =
+ is_default_constructible_v<typename _Nth_type<0, _Types...>::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<typename... _Types>
union _Variadic_union { };
@@ -355,6 +395,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Variant_storage()
{ _M_reset(); }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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;
@@ -374,59 +427,108 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void _M_reset()
{ _M_index = variant_npos; }
+ void*
+ _M_storage() const
+ {
+ return const_cast<void*>(static_cast<const void*>(
+ 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<typename... _Types>
- 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<bool, typename... _Types>
+ 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<typename... _Types>
+ struct _Copy_ctor_base<true, _Types...> : _Variant_storage_alias<_Types...>
+ {
+ using _Base = _Variant_storage_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_ctor_alias =
+ _Copy_ctor_base<_Traits<_Types...>::_S_trivial_copy_ctor, _Types...>;
+
+ template<bool, typename... _Types>
+ 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<size_t _Np, typename... _Args>
- 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<typename... _Types>
+ struct _Move_ctor_base<true, _Types...> : _Copy_ctor_alias<_Types...>
+ {
+ using _Base = _Copy_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_ctor_alias =
+ _Move_ctor_base<_Traits<_Types...>::_S_trivial_move_ctor, _Types...>;
+
+ template<bool, typename... _Types>
+ 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)
{
@@ -434,16 +536,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 (...)
{
@@ -455,12 +557,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<typename... _Types>
+ struct _Copy_assign_base<true, _Types...> : _Move_ctor_alias<_Types...>
+ {
+ using _Base = _Move_ctor_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Copy_assign_alias =
+ _Copy_assign_base<_Traits<_Types...>::_S_trivial_copy_assign,
+ _Types...>;
+
+ template<bool, typename... _Types>
+ 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 (...)
{
@@ -469,40 +594,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<void*>(static_cast<const void*>(
- 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<typename... _Types>
+ struct _Move_assign_base<true, _Types...> : _Copy_assign_alias<_Types...>
+ {
+ using _Base = _Copy_assign_alias<_Types...>;
+ using _Base::_Base;
+ };
+
+ template<typename... _Types>
+ using _Move_assign_alias =
+ _Move_assign_base<_Traits<_Types...>::_S_trivial_move_assign,
+ _Types...>;
+
+ template<typename... _Types>
+ 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<size_t _Np, typename... _Args>
+ 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?
@@ -877,16 +1036,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:
@@ -899,9 +1055,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<typename _Tp>
static constexpr bool
@@ -928,12 +1084,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<typename _Tp,
typename = enable_if_t<!is_same_v<decay_t<_Tp>, variant>>,
@@ -942,7 +1101,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<typename _Tp, typename... _Args,
@@ -950,7 +1110,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& is_constructible_v<_Tp, _Args&&...>>>
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<typename _Tp, typename _Up, typename... _Args,
@@ -983,13 +1144,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Default_ctor_enabler(_Enable_default_constructor_tag{})
{ __glibcxx_assert(index() == _Np); }
- ~variant() = default;
-
- variant& operator=(const variant&) = default;
- variant& operator=(variant&&)
- noexcept((is_nothrow_move_constructible_v<_Types> && ...)
- && (is_nothrow_move_assignable_v<_Types> && ...)) = default;
-
template<typename _Tp>
enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
&& is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
@@ -1084,7 +1238,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<variant<int, string>>, "");
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>>, "");
{
variant<int> a;
- static_assert(!noexcept(variant<int>(a)), "");
+ static_assert(noexcept(variant<int>(a)), "");
}
{
variant<string> a;
@@ -103,7 +105,7 @@ void copy_ctor()
}
{
variant<int, char> a;
- static_assert(!noexcept(variant<int, char>(a)), "");
+ static_assert(noexcept(variant<int, char>(a)), "");
}
}
@@ -111,6 +113,8 @@ void move_ctor()
{
static_assert(is_move_constructible_v<variant<int, string>>, "");
static_assert(!is_move_constructible_v<variant<AllDeleted, string>>, "");
+ 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>>())), "");
static_assert(noexcept(variant<int, DefaultNoexcept>(declval<variant<int, DefaultNoexcept>>())), "");
}
@@ -148,13 +152,15 @@ void copy_assign()
{
static_assert(is_copy_assignable_v<variant<int, string>>, "");
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>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = a), "");
}
{
variant<DefaultNoexcept> a;
- static_assert(!noexcept(a = a), "");
+ static_assert(noexcept(a = a), "");
}
}
@@ -162,6 +168,8 @@ void move_assign()
{
static_assert(is_move_assignable_v<variant<int, string>>, "");
static_assert(!is_move_assignable_v<variant<AllDeleted, string>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<int>>, "");
+ static_assert(!is_trivially_move_assignable_v<variant<string>>, "");
{
variant<Empty> a;
static_assert(!noexcept(a = std::move(a)), "");
@@ -454,3 +462,92 @@ void test_emplace()
static_assert(!has_type_emplace<variant<AllDeleted>, AllDeleted>(0), "");
static_assert(!has_index_emplace<variant<AllDeleted>, 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<variant<A>>, ""); \
+ static_assert(MC_VAL == is_trivially_move_constructible_v<variant<A>>, ""); \
+ static_assert(CA_VAL == is_trivially_copy_assignable_v<variant<A>>, ""); \
+ static_assert(MA_VAL == is_trivially_move_assignable_v<variant<A>>, ""); \
+ }
+ 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<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_constructible_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_copy_assignable_v<variant<AllDeleted, A>>, ""); \
+ static_assert(!is_trivially_move_assignable_v<variant<AllDeleted, A>>, ""); \
+ }
+ 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<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_constructible_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_copy_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+ static_assert(is_trivially_move_assignable_v<variant<DefaultNoexcept, int, char, float, double>>, "");
+}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Patch] Forward triviality in variant
2017-06-18 19:37 ` Tim Shen via gcc-patches
@ 2017-06-27 15:43 ` Jonathan Wakely
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Wakely @ 2017-06-27 15:43 UTC (permalink / raw)
To: Tim Shen; +Cc: Ville Voutilainen, libstdc++, gcc-patches
On 18/06/17 12:37 -0700, Tim Shen via libstdc++ wrote:
>Besides the changes on the comments, I also changed the definition of
>_S_trivial_copy_assign and _S_trivial_move_assign to match what union
>has. See [class.copy.assign]p9.
>
>On Thu, Jun 1, 2017 at 8:13 AM, Jonathan Wakely wrote:
>> On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>>>
>>> diff --git a/libstdc++-v3/include/std/variant
>>> b/libstdc++-v3/include/std/variant
>>> index b9824a5182c..f81b815af09 100644
>>> --- a/libstdc++-v3/include/std/variant
>>> +++ b/libstdc++-v3/include/std/variant
>>> @@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>> __ref_cast<_Tp>(__t));
>>> }
>>>
>>> + template<typename... _Types>
>>> + struct _Traits
>>> + {
>>> + static constexpr bool is_default_constructible_v =
>>> + is_default_constructible_v<typename _Nth_type<0,
>>> _Types...>::type>;
>>> + static constexpr bool is_copy_constructible_v =
>>> + __and_<is_copy_constructible<_Types>...>::value;
>>> + static constexpr bool is_move_constructible_v =
>>> + __and_<is_move_constructible<_Types>...>::value;
>>> + static constexpr bool is_copy_assignable_v =
>>> + is_copy_constructible_v && is_move_constructible_v
>>> + && __and_<is_copy_assignable<_Types>...>::value;
>>> + static constexpr bool is_move_assignable_v =
>>> + is_move_constructible_v
>>> + && __and_<is_move_assignable<_Types>...>::value;
>>
>>
>> It seems strange to me that these ones end with _v but the following
>> ones don't. Could we make them all have no _v suffix?
>
>Done. They are internal traits only for readability, so I shortened
>the names and make them libstdc++ style, e.g. _S_copy_ctor.
>
>>
>>> + static constexpr bool is_dtor_trivial =
>>> + __and_<is_trivially_destructible<_Types>...>::value;
>>> + static constexpr bool is_copy_ctor_trivial =
>>> + __and_<is_trivially_copy_constructible<_Types>...>::value;
>>> + static constexpr bool is_move_ctor_trivial =
>>> + __and_<is_trivially_move_constructible<_Types>...>::value;
>>> + static constexpr bool is_copy_assign_trivial =
>>> + is_dtor_trivial
>>> + && is_copy_ctor_trivial
>>> + && __and_<is_trivially_copy_assignable<_Types>...>::value;
>>> + static constexpr bool is_move_assign_trivial =
>>> + is_dtor_trivial
>>> + && is_move_ctor_trivial
>>> + && __and_<is_trivially_move_assignable<_Types>...>::value;
>>> +
>>> + static constexpr bool is_default_ctor_noexcept =
>>> + is_nothrow_default_constructible_v<
>>> + typename _Nth_type<0, _Types...>::type>;
>>> + static constexpr bool is_copy_ctor_noexcept =
>>> + is_copy_ctor_trivial;
>>> + static constexpr bool is_move_ctor_noexcept =
>>> + is_move_ctor_trivial
>>> + || __and_<is_nothrow_move_constructible<_Types>...>::value;
>>> + static constexpr bool is_copy_assign_noexcept =
>>> + is_copy_assign_trivial;
>>> + static constexpr bool is_move_assign_noexcept =
>>> + is_move_assign_trivial ||
>>> + (is_move_ctor_noexcept
>>> + && __and_<is_nothrow_move_assignable<_Types>...>::value);
>>> + };
>>
>>
>> Does using __and_ for any of those traits reduce the limit on the
>> number of alternatives in a variant? We switched to using fold
>> expressions in some contexts to avoid very deep instantiations, but I
>> don't know if these will hit the same problem, but it looks like it
>> will.
>
>Done, use fold expression instead. At one point we changed some fold
>expressions to __and_, because __and_ has short circuiting; does fold
>expressions have short circuits too? Now that I think about it, short
>circuiting in a constant fold expression should be a QoI issue.
Fold expressions don't short-circuit ... I'm not sure if they would be
allowed to for QoI.
>>> @@ -928,12 +1107,19 @@ _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() noexcept(_Traits::is_default_ctor_noexcept) = default;
>>
>>
>> Do we need the exception specifications here? Will the =default make
>> the right thing happen anyway? (And if not, won't we get an error by
>> trying to define the constructors as noexcept when the implicit
>> definition would not be noexcept?)
>
>Done. Removed unnecessary noexcept qualifiers.
>
>It turns out I mistakenly thought using "variant() = default" means
>`variant() noexcept(false) = default`.
OK for trunk, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-06-27 15:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-30 6:36 [Patch] Forward triviality in variant Tim Shen via gcc-patches
2017-05-30 9:41 ` Tim Shen via gcc-patches
2017-06-01 15:13 ` Jonathan Wakely
2017-06-01 15:21 ` Ville Voutilainen
2017-06-01 15:29 ` Jonathan Wakely
2017-06-01 15:43 ` Ville Voutilainen
2017-06-01 16:03 ` Jonathan Wakely
2017-06-01 16:07 ` Ville Voutilainen
2017-06-01 16:13 ` Jonathan Wakely
2017-06-18 19:37 ` Tim Shen via gcc-patches
2017-06-27 15:43 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).