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.
