From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63579 invoked by alias); 27 Jun 2017 15:43:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 63548 invoked by uid 89); 27 Jun 2017 15:43:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_SORBS_SPAM,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=HTo:U*timshen X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Jun 2017 15:43:47 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 51328334594; Tue, 27 Jun 2017 15:43:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 51328334594 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jwakely@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 51328334594 Received: from localhost (unknown [10.33.36.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id EF08F843B4; Tue, 27 Jun 2017 15:43:45 +0000 (UTC) Date: Tue, 27 Jun 2017 15:43:00 -0000 From: Jonathan Wakely To: Tim Shen Cc: Ville Voutilainen , libstdc++ , gcc-patches Subject: Re: [Patch] Forward triviality in variant Message-ID: <20170627154345.GX5211@redhat.com> References: <20170601151302.GA10443@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.8.0 (2017-02-23) X-SW-Source: 2017-06/txt/msg02060.txt.bz2 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 >>> + struct _Traits >>> + { >>> + static constexpr bool is_default_constructible_v = >>> + is_default_constructible_v>> _Types...>::type>; >>> + static constexpr bool is_copy_constructible_v = >>> + __and_...>::value; >>> + static constexpr bool is_move_constructible_v = >>> + __and_...>::value; >>> + static constexpr bool is_copy_assignable_v = >>> + is_copy_constructible_v && is_move_constructible_v >>> + && __and_...>::value; >>> + static constexpr bool is_move_assignable_v = >>> + is_move_constructible_v >>> + && __and_...>::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_...>::value; >>> + static constexpr bool is_copy_ctor_trivial = >>> + __and_...>::value; >>> + static constexpr bool is_move_ctor_trivial = >>> + __and_...>::value; >>> + static constexpr bool is_copy_assign_trivial = >>> + is_dtor_trivial >>> + && is_copy_ctor_trivial >>> + && __and_...>::value; >>> + static constexpr bool is_move_assign_trivial = >>> + is_dtor_trivial >>> + && is_move_ctor_trivial >>> + && __and_...>::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_...>::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_...>::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.