[Patch] SFINAE on is_same first in variant's _Tp&& constructor

[Jonathan Wakely]

On 22/05/17 16:14 -0400, Tim Song wrote:
>On Mon, May 22, 2017 at 9:21 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 19/05/17 22:40 -0700, Tim Shen via libstdc++ wrote:
>>>
>>> diff --git a/libstdc++-v3/include/std/variant
>>> b/libstdc++-v3/include/std/variant
>>> index 0e04a820d69..b9824a5182c 100644
>>> --- a/libstdc++-v3/include/std/variant
>>> +++ b/libstdc++-v3/include/std/variant
>>> @@ -936,9 +936,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>       noexcept((is_nothrow_move_constructible_v<_Types> && ...)) =
>>> default;
>>>
>>>       template<typename _Tp,
>>> +              typename = enable_if_t<!is_same_v<decay_t<_Tp>, variant>>,
>>>                typename =
>>> enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
>>> -                         && is_constructible_v<__accepted_type<_Tp&&>,
>>> _Tp&&>
>>> -                         && !is_same_v<decay_t<_Tp>, variant>>>
>>> +                         && is_constructible_v<__accepted_type<_Tp&&>,
>>> _Tp&&>>>
>>
>>
>> Does this definitely short-circuit? I seem to recall a similar case
>> where either Clang or GCC (I think it was Clang) was evaluating the
>> second default template argument even though the first had produce a
>> substition failure.
>>
>> If we need to guarantee it short-circuits then we'd want:
>>
>>      template<typename _Tp,
>>                typename = enable_if_t<__and_<
>>                                      __not_<is_same<decay_t<_Tp>, variant>>,
>>                                      __bool_constant<
>>                                        __exactly_once<__accepted_type<_Tp&&>>
>>                                        && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>>>
>>
>> i.e. use __and_< is-this-type, everything-else> where
>> "everything-else" still uses && to avoid making the instantiations too
>> deep.
>>
>> Also, this is another place where we could use an __is_samey<T, U>
>> trait that does is_same<remove_cv_t<remove_reference_t<T>, U>.
>>
>
>The thing that needs to be short-circuited out is the evaluation of
>__accepted_type<_Tp&&>, which starts the tower of turtles.

Ah I see.

>The original patch does that (assuming core issue 1227's resolution),
>but the __and_ version doesn't; __and_ only short circuits the
>immediate parameter, not things used in forming it.

Then the original patch is OK for trunk and gcc-7-branch.

Thank you Tim and Tim for the explanations.