[Patch] Forward triviality in variant

[Tim Shen via libstdc++]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: c.diff
Type: text/x-patch
Size: 24736 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20170618/1b8dd71a/attachment.bin>