From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 0115A3858030 for ; Fri, 5 Nov 2021 21:19:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0115A3858030 Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-260-eA6fKNA_Oky1310-9njAgQ-1; Fri, 05 Nov 2021 17:19:50 -0400 X-MC-Unique: eA6fKNA_Oky1310-9njAgQ-1 Received: by mail-ed1-f71.google.com with SMTP id v9-20020a50d849000000b003dcb31eabaaso10068043edj.13 for ; Fri, 05 Nov 2021 14:19:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P+jddvJIpl+X0m39t2zEdC5EviFQ9EdvyVlNtuPL3bk=; b=anO6yk5P8JfNy5NZ7Iusr/EFK5LbVSNo/q21h/IpLSIGD5RUr6vf6l6z0lNAlb++7P AZrpLLFo5RBFQ0ye/H59DSXTogIX4FEDwFQfFtZYhB6e9b08WdWdjfngsMAKeE6f/J3H v7USSgVsrq9tD/Iq8NSNEjxEb9bPr3jIK1EDHC3CDvBNI2yhqpOP/b/LPwYjk4jbAnss AhydHkqfZYubjmoughPd5KfmfECR+ULkqROsTbdz2KRkd9jwXUBDJ2h+mzcZyVjKqaSA t7KSxPN04K/U/60AEg0XANDCJcYhgscEuUSytkFKejwQdJ5FKgOc+adru/yyIvGSTd4J hs0Q== X-Gm-Message-State: AOAM5316sxae3mDgTqyf4X/X9r1mVkKEPLx8tQAeX5Q9yVYk1fC9ZXkZ RqdGTExCKpL8jWzFqJz4qqbBwPD8NKCbruzuvsnPBJGU3tgG3DOS/m6ZMJ1TwoHGg+5thrECnjS mqQytbND4zcGQJcQ/ZGpCRSeyBELgonM= X-Received: by 2002:a17:906:c005:: with SMTP id e5mr74183029ejz.480.1636147189051; Fri, 05 Nov 2021 14:19:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzFm866luf9DmtErK8yEwJxGgZ6W1luO2oG2gaz7mTc9h8xzndlQQDIVC7S6E5mryFugRIWsxw9URo66XIt758= X-Received: by 2002:a17:906:c005:: with SMTP id e5mr74182998ejz.480.1636147188588; Fri, 05 Nov 2021 14:19:48 -0700 (PDT) MIME-Version: 1.0 References: <20211104183322.2123606-1-jwakely@redhat.com> In-Reply-To: <20211104183322.2123606-1-jwakely@redhat.com> From: Patrick Palka Date: Fri, 5 Nov 2021 17:19:37 -0400 Message-ID: Subject: Re: [committed] libstdc++: Optimize std::tuple_element and std::tuple_size_v To: Jonathan Wakely Cc: "Jonathan Wakely via Libstdc++" , GCC Patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Nov 2021 21:19:54 -0000 On Thu, Nov 4, 2021 at 2:34 PM Jonathan Wakely via Libstdc++ wrote: > > Tested powerpc64le-linux, committed to trunk. > > > This reduces the number of class template instantiations needed for code > using tuples, by reusing _Nth_type in tuple_element and specializing > tuple_size_v for tuple, pair and array (and const-qualified versions of > them). > > Also define the _Nth_type primary template as a complete type (but with > no nested 'type' member). This avoids "invalid use of incomplete type" > errors for out-of-range specializations of tuple_element. Those errors > would probably be confusing and unhelpful for users. We already have > a user-friendly static assert in tuple_element itself. > > Also ensure that tuple_size_v is available whenever tuple_size is (as > proposed by LWG 3387). We already do that for tuple_element_t. > > libstdc++-v3/ChangeLog: > > * include/bits/stl_pair.h (tuple_size_v): Define partial > specializations for std::pair. > * include/bits/utility.h (_Nth_type): Move definition here > and define primary template. > (tuple_size_v): Move definition here. > * include/std/array (tuple_size_v): Define partial > specializations for std::array. > * include/std/tuple (tuple_size_v): Move primary template to > . Define partial specializations for > std::tuple. > (tuple_element): Change definition to use _Nth_type. > * include/std/variant (_Nth_type): Move to . > (variant_alternative, variant): Adjust qualification of > _Nth_type. > * testsuite/20_util/tuple/element_access/get_neg.cc: Prune > additional errors from _Nth_type. > --- > libstdc++-v3/include/bits/stl_pair.h | 8 +++ > libstdc++-v3/include/bits/utility.h | 51 +++++++++++++++- > libstdc++-v3/include/std/array | 8 +++ > libstdc++-v3/include/std/tuple | 39 +++++-------- > libstdc++-v3/include/std/variant | 58 ++----------------- > .../20_util/tuple/element_access/get_neg.cc | 1 + > 6 files changed, 84 insertions(+), 81 deletions(-) > > diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h > index 5b400daf97f..6081e0c7fe9 100644 > --- a/libstdc++-v3/include/bits/stl_pair.h > +++ b/libstdc++-v3/include/bits/stl_pair.h > @@ -771,6 +771,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > struct tuple_element<1, pair<_Tp1, _Tp2>> > { typedef _Tp2 type; }; > > +#if __cplusplus >= 201703L > + template > + inline constexpr size_t tuple_size_v> = 2; > + > + template > + inline constexpr size_t tuple_size_v> = 2; > +#endif > + > /// @cond undocumented > template > struct __pair_get; > diff --git a/libstdc++-v3/include/bits/utility.h b/libstdc++-v3/include/bits/utility.h > index fce52a4530d..c9ffa008217 100644 > --- a/libstdc++-v3/include/bits/utility.h > +++ b/libstdc++-v3/include/bits/utility.h > @@ -70,6 +70,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > struct tuple_size> > : public tuple_size<_Tp> { }; > > +#if __cplusplus >= 201703L > + template > + inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value; > +#endif > + > /// Gives the type of the ith element of a given tuple type. > template > struct tuple_element; > @@ -97,8 +102,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > }; > > #if __cplusplus >= 201402L > -// The standard says this macro and alias template should be in > -// but we define them here, to be available in and too. > +// The standard says this macro and alias template should be in but we > +// we define them here, to be available in , and too. > +// _GLIBCXX_RESOLVE_LIB_DEFECTS > +// 3378. tuple_size_v/tuple_element_t should be available when > +// tuple_size/tuple_element are > #define __cpp_lib_tuple_element_t 201402L > > template > @@ -195,6 +203,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #endif // C++17 > #endif // C++14 > > + template > + struct _Nth_type > + { }; > + > + template > + struct _Nth_type<0, _Tp0, _Rest...> > + { using type = _Tp0; }; > + > + template > + struct _Nth_type<1, _Tp0, _Tp1, _Rest...> > + { using type = _Tp1; }; > + > + template > + struct _Nth_type<2, _Tp0, _Tp1, _Tp2, _Rest...> > + { using type = _Tp2; }; > + > + template + typename... _Rest> > +#if __cpp_concepts > + requires (_Np >= 3) > +#endif > + struct _Nth_type<_Np, _Tp0, _Tp1, _Tp2, _Rest...> > + : _Nth_type<_Np - 3, _Rest...> > + { }; > + > +#if ! __cpp_concepts // Need additional specializations to avoid ambiguities. > + template > + struct _Nth_type<0, _Tp0, _Tp1, _Rest...> > + { using type = _Tp0; }; > + > + template > + struct _Nth_type<0, _Tp0, _Tp1, _Tp2, _Rest...> > + { using type = _Tp0; }; > + > + template > + struct _Nth_type<1, _Tp0, _Tp1, _Tp2, _Rest...> > + { using type = _Tp1; }; > +#endif > + > _GLIBCXX_END_NAMESPACE_VERSION > } // namespace > > diff --git a/libstdc++-v3/include/std/array b/libstdc++-v3/include/std/array > index 3e12d35157c..413f8e2be01 100644 > --- a/libstdc++-v3/include/std/array > +++ b/libstdc++-v3/include/std/array > @@ -481,6 +481,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > using type = _Tp; > }; > > +#if __cplusplus >= 201703L > + template > + inline constexpr size_t tuple_size_v> = _Nm; > + > + template > + inline constexpr size_t tuple_size_v> = _Nm; > +#endif > + > template > struct __is_tuple_like_impl> : true_type > { }; > diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple > index aaee0b8826a..b82cdf12569 100644 > --- a/libstdc++-v3/include/std/tuple > +++ b/libstdc++-v3/include/std/tuple > @@ -1344,36 +1344,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > struct tuple_size> > : public integral_constant { }; > > -#if __cplusplus > 201402L > - template > - inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value; > +#if __cplusplus >= 201703L > + template > + inline constexpr size_t tuple_size_v> Missing ... after _Types? > + = sizeof...(_Types); > + > + template > + inline constexpr size_t tuple_size_v> Same here. (I guess now's a good time to fix PR100652!) > + = sizeof...(_Types); > #endif > > - /** > - * Recursive case for tuple_element: strip off the first element in > - * the tuple and retrieve the (i-1)th element of the remaining tuple. > - */ > - template > - struct tuple_element<__i, tuple<_Head, _Tail...> > > - : tuple_element<__i - 1, tuple<_Tail...> > { }; > - > - /** > - * Basis case for tuple_element: The first element is the one we're seeking. > - */ > - template > - struct tuple_element<0, tuple<_Head, _Tail...> > > + /// Trait to get the Ith element type from a tuple. > + template > + struct tuple_element<__i, tuple<_Types...>> > { > - typedef _Head type; > - }; > + static_assert(__i < sizeof...(_Types), "tuple index must be in range"); > > - /** > - * Error case for tuple_element: invalid index. > - */ > - template > - struct tuple_element<__i, tuple<>> > - { > - static_assert(__i < tuple_size>::value, > - "tuple index must be in range"); > + using type = typename _Nth_type<__i, _Types...>::type; > }; > > template > diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant > index c4c307b7bb2..993ce3dba91 100644 > --- a/libstdc++-v3/include/std/variant > +++ b/libstdc++-v3/include/std/variant > @@ -54,51 +54,6 @@ namespace std _GLIBCXX_VISIBILITY(default) > { > _GLIBCXX_BEGIN_NAMESPACE_VERSION > > -namespace __detail > -{ > -namespace __variant > -{ > - template > - struct _Nth_type; > - > - template > - struct _Nth_type<0, _Tp0, _Rest...> > - { using type = _Tp0; }; > - > - template > - struct _Nth_type<1, _Tp0, _Tp1, _Rest...> > - { using type = _Tp1; }; > - > - template > - struct _Nth_type<2, _Tp0, _Tp1, _Tp2, _Rest...> > - { using type = _Tp2; }; > - > - template - typename... _Rest> > -#if __cpp_concepts > - requires (_Np >= 3) > -#endif > - struct _Nth_type<_Np, _Tp0, _Tp1, _Tp2, _Rest...> > - : _Nth_type<_Np - 3, _Rest...> > - { }; > - > -#if ! __cpp_concepts // Need additional specializations to avoid ambiguities. > - template > - struct _Nth_type<0, _Tp0, _Tp1, _Rest...> > - { using type = _Tp0; }; > - > - template > - struct _Nth_type<0, _Tp0, _Tp1, _Tp2, _Rest...> > - { using type = _Tp0; }; > - > - template > - struct _Nth_type<1, _Tp0, _Tp1, _Tp2, _Rest...> > - { using type = _Tp1; }; > -#endif > - > -} // namespace __variant > -} // namespace __detail > - > #if __cplusplus >= 202002L && __cpp_concepts > // P2231R1 constexpr needs constexpr unions and constrained destructors. > # define __cpp_lib_variant 202106L > @@ -145,8 +100,7 @@ namespace __variant > { > static_assert(_Np < sizeof...(_Types)); > > - using type > - = typename __detail::__variant::_Nth_type<_Np, _Types...>::type; > + using type = typename _Nth_type<_Np, _Types...>::type; > }; > > template > @@ -1442,8 +1396,7 @@ namespace __variant > = __detail::__variant::__accepted_index<_Tp, variant>::value; > > template> > - using __to_type > - = typename __detail::__variant::_Nth_type<_Np, _Types...>::type; > + using __to_type = typename _Nth_type<_Np, _Types...>::type; > > template>> > using __accepted_type = __to_type<__accepted_index<_Tp>>; > @@ -1580,7 +1533,7 @@ namespace __variant > emplace(_Args&&... __args) > { > namespace __variant = std::__detail::__variant; > - using type = typename __variant::_Nth_type<_Np, _Types...>::type; > + using type = typename _Nth_type<_Np, _Types...>::type; > // Provide the strong exception-safety guarantee when possible, > // to avoid becoming valueless. > if constexpr (is_nothrow_constructible_v) > @@ -1620,7 +1573,7 @@ namespace __variant > emplace(initializer_list<_Up> __il, _Args&&... __args) > { > namespace __variant = std::__detail::__variant; > - using type = typename __variant::_Nth_type<_Np, _Types...>::type; > + using type = typename _Nth_type<_Np, _Types...>::type; > // Provide the strong exception-safety guarantee when possible, > // to avoid becoming valueless. > if constexpr (is_nothrow_constructible_v @@ -1803,8 +1756,7 @@ namespace __variant > constexpr size_t __max = 11; // "These go to eleven." > > // The type of the first variant in the pack. > - using _V0 > - = typename __detail::__variant::_Nth_type<0, _Variants...>::type; > + using _V0 = typename _Nth_type<0, _Variants...>::type; > // The number of alternatives in that first variant. > constexpr auto __n = variant_size_v>; > > diff --git a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc > index 225bb6245a6..113a7fd62de 100644 > --- a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc > +++ b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc > @@ -60,3 +60,4 @@ test03() > } > > // { dg-error "tuple index must be in range" "" { target *-*-* } 0 } > +// { dg-prune-output "no type named 'type' in .*_Nth_type" } > -- > 2.31.1 >