public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: "Jonathan Wakely via Libstdc++" <libstdc++@gcc.gnu.org>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [committed] libstdc++: Optimize std::tuple_element and std::tuple_size_v
Date: Fri, 5 Nov 2021 21:45:54 +0000	[thread overview]
Message-ID: <CACb0b4kTK3+ZeKYE+RXM4j7G+iEXtnw2WGM=EW2N8BW=yqEuEw@mail.gmail.com> (raw)
In-Reply-To: <CAMOnLZZNjdP6_BW5mgb5zJVT936OxV63WTkdBFrpr-mQyKgR1A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7789 bytes --]

On Fri, 5 Nov 2021 at 21:19, Patrick Palka <ppalka@redhat.com> wrote:

> On Thu, Nov 4, 2021 at 2:34 PM Jonathan Wakely via Libstdc++
> <libstdc++@gcc.gnu.org> 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
> >         <bits/utility.h>.  Define partial specializations for
> >         std::tuple.
> >         (tuple_element): Change definition to use _Nth_type.
> >         * include/std/variant (_Nth_type): Move to <bits/utility.h>.
> >         (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<typename _Tp1, typename _Tp2>
> > +    inline constexpr size_t tuple_size_v<pair<_Tp1, _Tp2>> = 2;
> > +
> > +  template<typename _Tp1, typename _Tp2>
> > +    inline constexpr size_t tuple_size_v<const pair<_Tp1, _Tp2>> = 2;
> > +#endif
> > +
> >    /// @cond undocumented
> >    template<size_t _Int>
> >      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<const volatile __enable_if_has_tuple_size<_Tp>>
> >      : public tuple_size<_Tp> { };
> >
> > +#if __cplusplus >= 201703L
> > +  template<typename _Tp>
> > +    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<size_t __i, typename _Tp>
> >      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 <tuple>
> > -// but we define them here, to be available in <utility> and <array>
> too.
> > +// The standard says this macro and alias template should be in <tuple>
> but we
> > +// we define them here, to be available in <array>, <utility> and
> <ranges> 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<size_t __i, typename _Tp>
> > @@ -195,6 +203,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >  #endif // C++17
> >  #endif // C++14
> >
> > +  template<size_t _Np, typename... _Types>
> > +    struct _Nth_type
> > +    { };
> > +
> > +  template<typename _Tp0, typename... _Rest>
> > +    struct _Nth_type<0, _Tp0, _Rest...>
> > +    { using type = _Tp0; };
> > +
> > +  template<typename _Tp0, typename _Tp1, typename... _Rest>
> > +    struct _Nth_type<1, _Tp0, _Tp1, _Rest...>
> > +    { using type = _Tp1; };
> > +
> > +  template<typename _Tp0, typename _Tp1, typename _Tp2, typename...
> _Rest>
> > +    struct _Nth_type<2, _Tp0, _Tp1, _Tp2, _Rest...>
> > +    { using type = _Tp2; };
> > +
> > +  template<size_t _Np, typename _Tp0, typename _Tp1, typename _Tp2,
> > +          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<typename _Tp0, typename _Tp1, typename... _Rest>
> > +    struct _Nth_type<0, _Tp0, _Tp1, _Rest...>
> > +    { using type = _Tp0; };
> > +
> > +  template<typename _Tp0, typename _Tp1, typename _Tp2, typename...
> _Rest>
> > +    struct _Nth_type<0, _Tp0, _Tp1, _Tp2, _Rest...>
> > +    { using type = _Tp0; };
> > +
> > +  template<typename _Tp0, typename _Tp1, typename _Tp2, typename...
> _Rest>
> > +    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<typename _Tp, size_t _Nm>
> > +    inline constexpr size_t tuple_size_v<array<_Tp, _Nm>> = _Nm;
> > +
> > +  template<typename _Tp, size_t _Nm>
> > +    inline constexpr size_t tuple_size_v<const array<_Tp, _Nm>> = _Nm;
> > +#endif
> > +
> >    template<typename _Tp, size_t _Nm>
> >      struct __is_tuple_like_impl<array<_Tp, _Nm>> : 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<tuple<_Elements...>>
> >      : public integral_constant<size_t, sizeof...(_Elements)> { };
> >
> > -#if __cplusplus > 201402L
> > -  template <typename _Tp>
> > -    inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value;
> > +#if __cplusplus >= 201703L
> > +  template<typename... _Types>
> > +    inline constexpr size_t tuple_size_v<tuple<_Types>>
>
> Missing ... after _Types?
>
> > +      = sizeof...(_Types);
> > +
> > +  template<typename... _Types>
> > +    inline constexpr size_t tuple_size_v<const tuple<_Types>>
>
> Same here.  (I guess now's a good time to fix PR100652!)
>
>
>
Oops!  Fixed by the attached patch, pushed to trunk. Thanks.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 954 bytes --]

commit 70d6f6e41f77269f7d293c6fbccf978680e68d2a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Nov 5 21:40:58 2021

    libstdc++: Fix pack expansions in tuple_size_v specializations
    
    libstdc++-v3/ChangeLog:
    
            * include/std/tuple (tuple_size_v): Fix pack expansion.

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 46173935b64..36dc05d97bc 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -1346,11 +1346,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus >= 201703L
   template<typename... _Types>
-    inline constexpr size_t tuple_size_v<tuple<_Types>>
+    inline constexpr size_t tuple_size_v<tuple<_Types...>>
       = sizeof...(_Types);
 
   template<typename... _Types>
-    inline constexpr size_t tuple_size_v<const tuple<_Types>>
+    inline constexpr size_t tuple_size_v<const tuple<_Types...>>
       = sizeof...(_Types);
 #endif
 

      reply	other threads:[~2021-11-05 21:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-04 18:33 Jonathan Wakely
2021-11-05 21:19 ` Patrick Palka
2021-11-05 21:45   ` Jonathan Wakely [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACb0b4kTK3+ZeKYE+RXM4j7G+iEXtnw2WGM=EW2N8BW=yqEuEw@mail.gmail.com' \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=ppalka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).