This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor
- From: "Tim Shen via libstdc++" <libstdc++ at gcc dot gnu dot org>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: "libstdc++" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 22 May 2017 11:05:33 -0700
- Subject: Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor
- Authentication-results: sourceware.org; auth=none
- References: <CAG4ZjNkZui6RkKB-cydS+q1=xUcuNS42Uha6w-N7fK7GbEOmSw@mail.gmail.com> <20170522132137.GH4527@redhat.com>
- Reply-to: Tim Shen <timshen at google dot com>
On Mon, May 22, 2017 at 6: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.
Good observation. I changed to use __and_ and __not_:
- typename = enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
- && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
- && !is_same_v<decay_t<_Tp>, variant>>>
+ typename = enable_if_t<__and_<
+ __not_<is_same<decay_t<_Tp>, variant>>,
+ std::integral_constant<bool,
+ __exactly_once<__accepted_type<_Tp&&>>>,
+ is_constructible<__accepted_type<_Tp&&>,
_Tp&&>>::value>>
(I didn't use && at all, just to verify the correctness)
but the compile still fails with the similar error messages. If __and_
and __not_ are expected to work, then the root cause is unlikely "not
short-circuit" only.
I suggest to cc a front-end person (Jason?) to take a look, as I
suggested in the bug, and the example: https://godbolt.org/g/AxUv16.
>
> 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>.
>
I never know that "samey" is a word. :)
--
Regards,
Tim Shen