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.129.124]) by sourceware.org (Postfix) with ESMTPS id E98D53858292 for ; Wed, 24 Jan 2024 15:24:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E98D53858292 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E98D53858292 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706109889; cv=none; b=Kg0rcw7mt4bI3Uoy3D3OOU2JSFUHbkSxDf8wlYbeSIolfSNLeBBiY7ZgmKUdzf53UJFETq6ZdkJa/4aGB+6qSd/50ngyxGLN17Ml/vkl4X8m83Lx43b49QDO+t0I8MgEGf6qe34GmY9GgtFfUCtlLezU4wgwxphTOeG6UPfIdZY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706109889; c=relaxed/simple; bh=JaRTXMlsdN0hN2liHSIyAvVqifqNH6eySj5TGKQWOTo=; h=DKIM-Signature:From:Date:To:Subject:Message-ID:MIME-Version; b=ck5giogE9YY7NaH1VfUHPWcRy1wUDDa2NhpkQqDerr3xcqaxLOh2R6JZQV1QRoAHB1grD6eVGw7IVRSYoNLRtDx3NHxwauHt/wKgIVBd8tt9Y5yNT4LhQspbOt5aSap0YVM3aT2YxcAlGhZcrOKtc4GVqU5lbs87gWrr2mqN9sI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706109886; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=qOvuvZH1NNHFoMpaesqtTk/0s2tJBn+lvz1+cNRCp1Q=; b=Lk/BtTM0I3/D+88ufO6jJVrpXvGYBjPq2cwMuplM8Yuthysa1hOw1qLOHCYBJuHzEtNxVD Cb4BCQksS0VfkISOUA0zWuwjGOr1ZmuyBwgyE8OeHZWEDit7+Foyzoe1Bzcxkvl3Sp+6Fm Lgp3FVbnCorBonMvA37vi7G951i6TXE= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-439-rtDSNsRrNPWIvwiEX76j0A-1; Wed, 24 Jan 2024 10:24:42 -0500 X-MC-Unique: rtDSNsRrNPWIvwiEX76j0A-1 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-7816e45f957so862365485a.1 for ; Wed, 24 Jan 2024 07:24:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706109882; x=1706714682; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=qOvuvZH1NNHFoMpaesqtTk/0s2tJBn+lvz1+cNRCp1Q=; b=xS4r9X60QSsD97bB/nRr+2SRb4zIdsXYIAYBzp7lHedA3La8PoXRreC5ImCB6L3DIR Qv7AFR2yA4+wrIktH3KNc1WtGe0I8qIvXfvO9iKdcDTBxHESAYRrmjwMwtvhyWzGbLRc +asvIsKjGWsIWw4I038IXt0yhXx+nYFa45b2en0xzkTytywXKVj7kCt9+B9Kb7BqsQBt cqbm43qUI20ExuCTN4fCC4E9ZbrcjJX0yyz64YXcQKAaT4pB1apuiduTBr4Z3JLImkwP yLeL7gQTvGeTUHV73ch5hVBH/d8vwdduIKUKuEHcN0+FNjzaJO3txrKZEdvYP1cqkqSe U3IA== X-Gm-Message-State: AOJu0YysCPBOtl8H/KbjdbT5AbafW1n+EfSAjWUFhyke4EMMCPc4VjZW xILtWN6DAJSnxXAMtksRcAORiDP+wpPyO/llcWmMaDazYbdJqq3p/cRshHeuv57dyE2Ow5VZ0dA 4OfL296VWK80RNaHiB3P/fF/8CAyG9S6fFVJuuObjXroYnh6Vh/zyHBw= X-Received: by 2002:a05:620a:898:b0:783:aebc:35f3 with SMTP id b24-20020a05620a089800b00783aebc35f3mr2352953qka.80.1706109881888; Wed, 24 Jan 2024 07:24:41 -0800 (PST) X-Google-Smtp-Source: AGHT+IEXusJe8TPHf6fOsodUrV1c9bHIPejjFJroWGeAIBh0UTIfwJ+qQRT5fkmj7E+/lrvxqWIGLw== X-Received: by 2002:a05:620a:898:b0:783:aebc:35f3 with SMTP id b24-20020a05620a089800b00783aebc35f3mr2352940qka.80.1706109881496; Wed, 24 Jan 2024 07:24:41 -0800 (PST) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id cx3-20020a05620a51c300b007831f5c6b65sm4113973qkb.47.2024.01.24.07.24.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Jan 2024 07:24:41 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Wed, 24 Jan 2024 10:24:40 -0500 (EST) To: Jonathan Wakely cc: Patrick Palka , gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Subject: Re: [PATCH 2/2] libstdc++: Implement P2165R4 changes to std::pair/tuple/etc In-Reply-To: Message-ID: References: <20240123235303.1540890-1-ppalka@redhat.com> <20240123235303.1540890-2-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-15.0 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_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 24 Jan 2024, Jonathan Wakely wrote: > On Tue, 23 Jan 2024 at 23:54, Patrick Palka wrote: > > diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h > > index b81b479ad43..a9b20fbe7ca 100644 > > --- a/libstdc++-v3/include/bits/stl_pair.h > > +++ b/libstdc++-v3/include/bits/stl_pair.h > > @@ -85,12 +85,70 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > /// @cond undocumented > > > > // Forward declarations. > > + template > > + struct pair; > > We have a compiler bug where a forward declaration without template > parameter names causes bad diagnostics later. The compiler seems to > try to use the parameter names from the first decl it sees, so we end > up with things like even when there's a name > available at the site of the actual error. So I think we should name > these _T1 and _T2 here. Will fix. > > > + > > template > > class tuple; > > > > + // Declarations of std::array and its std::get overloads, so that > > + // std::tuple_cat can use them if is included before . > > + // We also declare the other std::get overloads here so that they're > > + // visible to the P2165R4 tuple-like constructors of pair and tuple. > > + template > > + struct array; > > + > > template > > struct _Index_tuple; > > > > + template > > + constexpr typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type& > > + get(pair<_Tp1, _Tp2>& __in) noexcept; > > + > > + template > > + constexpr typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type&& > > + get(pair<_Tp1, _Tp2>&& __in) noexcept; > > + > > + template > > + constexpr const typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type& > > + get(const pair<_Tp1, _Tp2>& __in) noexcept; > > + > > + template > > + constexpr const typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type&& > > + get(const pair<_Tp1, _Tp2>&& __in) noexcept; > > + > > + template > > + constexpr __tuple_element_t<__i, tuple<_Elements...>>& > > + get(tuple<_Elements...>& __t) noexcept; > > + > > + template > > + constexpr const __tuple_element_t<__i, tuple<_Elements...>>& > > + get(const tuple<_Elements...>& __t) noexcept; > > + > > + template > > + constexpr __tuple_element_t<__i, tuple<_Elements...>>&& > > + get(tuple<_Elements...>&& __t) noexcept; > > + > > + template > > + constexpr const __tuple_element_t<__i, tuple<_Elements...>>&& > > + get(const tuple<_Elements...>&& __t) noexcept; > > + > > + template > > + constexpr _Tp& > > + get(array<_Tp, _Nm>&) noexcept; > > + > > + template > > + constexpr _Tp&& > > + get(array<_Tp, _Nm>&&) noexcept; > > + > > + template > > + constexpr const _Tp& > > + get(const array<_Tp, _Nm>&) noexcept; > > + > > + template > > + constexpr const _Tp&& > > + get(const array<_Tp, _Nm>&&) noexcept; > > + > > #if ! __cpp_lib_concepts > > // Concept utility functions, reused in conditionally-explicit > > // constructors. > > @@ -159,6 +217,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > #endif // lib concepts > > #endif // C++11 > > > > +#if __glibcxx_tuple_like // >= C++23 > > + template > > + inline constexpr bool __is_tuple_v = false; > > + > > + template > > + inline constexpr bool __is_tuple_v> = true; > > + > > + // TODO: Reuse __is_tuple_like from ? > > + template > > + inline constexpr bool __is_tuple_like_v = false; > > + > > + template > > + inline constexpr bool __is_tuple_like_v> = true; > > + > > + template > > + inline constexpr bool __is_tuple_like_v> = true; > > + > > + template > > + inline constexpr bool __is_tuple_like_v> = true; > > + > > + // __is_tuple_like_v is defined in . > > + > > + template > > + concept __tuple_like = __is_tuple_like_v>; > > + > > + template > > + concept __pair_like = __tuple_like<_Tp> && tuple_size_v> == 2; > > + > > + template > > + concept __eligible_tuple_like > > + = __detail::__different_from<_Tp, _Tuple> && __tuple_like<_Tp> > > + && (tuple_size_v> == tuple_size_v<_Tuple>) > > + && !ranges::__detail::__is_subrange>; > > + > > + template > > + concept __eligible_pair_like > > + = __detail::__different_from<_Tp, _Pair> && __pair_like<_Tp> > > + && !ranges::__detail::__is_subrange>; > > +#endif // C++23 > > + > > template class __pair_base > > { > > #if __cplusplus >= 201103L && ! __cpp_lib_concepts > > @@ -295,6 +393,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > return false; > > #endif > > } > > + > > +#if __glibcxx_tuple_like // >= C++23 > > + template > > + static constexpr bool > > + _S_constructible_from_pair_like() > > + { > > + return _S_constructible(std::declval<_UPair>())), > > + decltype(std::get<1>(std::declval<_UPair>()))>(); > > + } > > + > > + template > > + static constexpr bool > > + _S_convertible_from_pair_like() > > + { > > + return _S_convertible(std::declval<_UPair>())), > > + decltype(std::get<1>(std::declval<_UPair>()))>(); > > + } > > +#endif // C++23 > > /// @endcond > > > > public: > > @@ -393,6 +509,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > pair(const pair<_U1, _U2>&&) = delete; > > #endif // C++23 > > > > +#if __glibcxx_tuple_like // >= C++23 > > + template<__eligible_pair_like _UPair> > > + requires (_S_constructible_from_pair_like<_UPair>()) > > + constexpr explicit(!_S_convertible_from_pair_like<_UPair>()) > > + pair(_UPair&& __p) > > + : first(std::get<0>(std::forward<_UPair>(__p))), > > + second(std::get<1>(std::forward<_UPair>(__p))) > > + { } > > +#endif // C++23 > > I think this needs to be constrained with !_S_dangles<...>() and we > need a deleted overload with the same constraints, except for > _S_dangles being true. > > And that should be covered by a test. Oops, will fix. Should the deleted overloads carry over the conditionally explicit specifier? I noticed pair's deleted overloads do, but tuple's overloads don't. > > > > > diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple > > index be92f1eb973..182f3cc5e6a 100644 > > --- a/libstdc++-v3/include/std/tuple > > +++ b/libstdc++-v3/include/std/tuple > > @@ -50,6 +50,7 @@ > > #define __glibcxx_want_apply > > #define __glibcxx_want_make_from_tuple > > #define __glibcxx_want_ranges_zip > > +#define __glibcxx_want_tuple_like > > #include > > > > namespace std _GLIBCXX_VISIBILITY(default) > > @@ -246,6 +247,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > _Head _M_head_impl; > > }; > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + struct __tuple_like_tag_t { explicit __tuple_like_tag_t() = default; }; > > + > > + // Forward declared for use by the operator<=> overload for tuple-like types. > > + template > > + constexpr _Cat > > + __tuple_cmp(const _Tp&, const _Up&, index_sequence<>); > > + > > + template > + size_t _Idx0, size_t... _Idxs> > > + constexpr _Cat > > + __tuple_cmp(const _Tp& __t, const _Up& __u, > > + index_sequence<_Idx0, _Idxs...>); > > +#endif // C++23 > > + > > /** > > * Contains the actual implementation of the @c tuple template, stored > > * as a recursive inheritance hierarchy from the first element (most > > @@ -342,6 +358,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr > > + _Tuple_impl(__tuple_like_tag_t, _UTuple&& __u, index_sequence<_Is...>) > > + : _Tuple_impl(std::get<_Is>(std::forward<_UTuple>(__u))...) > > + { } > > +#endif // C++23 > > + > > template > > _GLIBCXX20_CONSTEXPR > > _Tuple_impl(allocator_arg_t __tag, const _Alloc& __a) > > @@ -428,6 +452,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr > > + _Tuple_impl(__tuple_like_tag_t, allocator_arg_t __tag, const _Alloc& __a, > > + _UTuple&& __u, index_sequence<_Is...>) > > + : _Tuple_impl(__tag, __a, std::get<_Is>(std::forward<_UTuple>(__u))...) > > + { } > > +#endif // C++23 > > + > > template > > _GLIBCXX20_CONSTEXPR > > void > > @@ -470,6 +503,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr void > > + _M_assign(__tuple_like_tag_t __tag, _UTuple&& __u) > > + { > > + _M_head(*this) = std::get<_Idx>(std::forward<_UTuple>(__u)); > > + _M_tail(*this)._M_assign(__tag, std::forward<_UTuple>(__u)); > > + } > > + > > + template > > + constexpr void > > + _M_assign(__tuple_like_tag_t __tag, _UTuple&& __u) const > > + { > > + _M_head(*this) = std::get<_Idx>(std::forward<_UTuple>(__u)); > > + _M_tail(*this)._M_assign(__tag, std::forward<_UTuple>(__u)); > > + } > > +#endif // C++23 > > + > > protected: > > _GLIBCXX20_CONSTEXPR > > void > > @@ -563,6 +614,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr > > + _Tuple_impl(__tuple_like_tag_t, _UTuple&& __u, index_sequence<0>) > > + : _Tuple_impl(std::get<0>(std::forward<_UTuple>(__u))) > > + { } > > +#endif // C++23 > > + > > template > > _GLIBCXX20_CONSTEXPR > > _Tuple_impl(allocator_arg_t __tag, const _Alloc& __a) > > @@ -633,6 +692,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr > > + _Tuple_impl(__tuple_like_tag_t, allocator_arg_t __tag, const _Alloc& __a, > > + _UTuple&& __u, index_sequence<0>) > > + : _Tuple_impl(__tag, __a, std::get<0>(std::forward<_UTuple>(__u))) > > + { } > > +#endif // C++23 > > + > > template > > _GLIBCXX20_CONSTEXPR > > void > > @@ -667,6 +735,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > } > > #endif // C++23 > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + constexpr void > > + _M_assign(__tuple_like_tag_t, _UTuple&& __u) > > + { _M_head(*this) = std::get<_Idx>(std::forward<_UTuple>(__u)); } > > + > > + template > > + constexpr void > > + _M_assign(__tuple_like_tag_t, _UTuple&& __u) const > > + { _M_head(*this) = std::get<_Idx>(std::forward<_UTuple>(__u)); } > > +#endif // C++23 > > + > > protected: > > _GLIBCXX20_CONSTEXPR > > void > > @@ -846,6 +926,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > #endif > > } > > > > +#if __cpp_lib_tuple_like // >= C++23 > > + template > > + static consteval bool > > + __constructible_from_tuple_like() > > + { > > + return [](index_sequence<_Is...>) { > > + return __constructible(std::declval<_UTuple>()))...>(); > > + }(make_index_sequence{}); > > + } > > + > > + template > > + static consteval bool > > + __convertible_from_tuple_like() > > + { > > + return [](index_sequence<_Is...>) { > > + return __convertible(std::declval<_UTuple>()))...>(); > > + }(make_index_sequence{}); > > These new functions can use index_sequence_for<_Elements...>{} here, > so you don't need the sizeof.... > That applies several times below as well. > > I think it's semantically identical, just a little shorter. I don't > know if there's any compilation speed benefit either way. Maybe > sizeof...(_Elements) is cheaper than expanding the pack into the > index_sequence_for alias template? > > > > + } > > +#endif // C++23 > > + > > public: > > constexpr > > explicit(!(__is_implicitly_default_constructible_v<_Elements> && ...)) > > @@ -1016,10 +1116,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > tuple(const pair<_U1, _U2>&&) = delete; > > #endif // C++23 > > > > -#if 0 && __cpp_lib_tuple_like // >= C++23 > > - template<__tuple_like _UTuple> > > - constexpr explicit(...) > > - tuple(_UTuple&& __u); > > +#if __cpp_lib_tuple_like // >= C++23 > > + template<__eligible_tuple_like _UTuple> > > + requires (__constructible_from_tuple_like<_UTuple>()) > > + && (!__use_other_ctor<_UTuple>()) > > + constexpr explicit(!__convertible_from_tuple_like<_UTuple>()) > > + tuple(_UTuple&& __u) > > + : _Inherited(__tuple_like_tag_t{}, > > + std::forward<_UTuple>(__u), > > + make_index_sequence{}) > > + { } > > #endif // C++23 > > > > // Allocator-extended constructors. > > @@ -1202,10 +1308,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > tuple(allocator_arg_t, const _Alloc&, const pair<_U1, _U2>&&) = delete; > > #endif // C++23 > > > > -#if 0 && __cpp_lib_tuple_like // >= C++23 > > - template > > - constexpr explicit(...) > > - tuple(allocator_arg_t __tag, const _Alloc& __a, _UTuple&& __u); > > +#if __cpp_lib_tuple_like // >= C++23 > > + template _UTuple> > > + requires (__constructible_from_tuple_like<_UTuple>()) > > + && (!__use_other_ctor<_UTuple>()) > > + constexpr explicit(!__convertible_from_tuple_like<_UTuple>()) > > + tuple(allocator_arg_t __tag, const _Alloc& __a, _UTuple&& __u) > > + : _Inherited(__tuple_like_tag_t{}, > > + __tag, __a, std::forward<_UTuple>(__u), > > + make_index_sequence{}) > > + { } > > #endif // C++23 > > For some reason these two new constructors aren't deleted if they > create dangling refs. I don't know why. Hmm, seems like an oversight. Shall we proactively implement them?