[Patch] Forward triviality in variant

[Jonathan Wakely]

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.