On Wed, 24 Jan 2024, Jonathan Wakely wrote:
> On Wed, 24 Jan 2024 at 15:24, Patrick Palka <ppa...@redhat.com> wrote:
> >
> > 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<typename, typename>
> > > > + 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 <template-argument-1-1> 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<typename...>
> > > > class tuple;
> > > >
> > > > + // Declarations of std::array and its std::get overloads, so that
> > > > + // std::tuple_cat can use them if <tuple> is included before <array>.
> > > > + // 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<typename _Tp, size_t _Nm>
> > > > + struct array;
> > > > +
> > > > template<size_t...>
> > > > struct _Index_tuple;
> > > >
> > > > + template<size_t _Int, class _Tp1, class _Tp2>
> > > > + constexpr typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type&
> > > > + get(pair<_Tp1, _Tp2>& __in) noexcept;
> > > > +
> > > > + template<size_t _Int, class _Tp1, class _Tp2>
> > > > + constexpr typename tuple_element<_Int, pair<_Tp1, _Tp2>>::type&&
> > > > + get(pair<_Tp1, _Tp2>&& __in) noexcept;
> > > > +
> > > > + template<size_t _Int, class _Tp1, class _Tp2>
> > > > + constexpr const typename tuple_element<_Int, pair<_Tp1,
> > > > _Tp2>>::type&
> > > > + get(const pair<_Tp1, _Tp2>& __in) noexcept;
> > > > +
> > > > + template<size_t _Int, class _Tp1, class _Tp2>
> > > > + constexpr const typename tuple_element<_Int, pair<_Tp1,
> > > > _Tp2>>::type&&
> > > > + get(const pair<_Tp1, _Tp2>&& __in) noexcept;
> > > > +
> > > > + template<size_t __i, typename... _Elements>
> > > > + constexpr __tuple_element_t<__i, tuple<_Elements...>>&
> > > > + get(tuple<_Elements...>& __t) noexcept;
> > > > +
> > > > + template<size_t __i, typename... _Elements>
> > > > + constexpr const __tuple_element_t<__i, tuple<_Elements...>>&
> > > > + get(const tuple<_Elements...>& __t) noexcept;
> > > > +
> > > > + template<size_t __i, typename... _Elements>
> > > > + constexpr __tuple_element_t<__i, tuple<_Elements...>>&&
> > > > + get(tuple<_Elements...>&& __t) noexcept;
> > > > +
> > > > + template<size_t __i, typename... _Elements>
> > > > + constexpr const __tuple_element_t<__i, tuple<_Elements...>>&&
> > > > + get(const tuple<_Elements...>&& __t) noexcept;
> > > > +
> > > > + template<size_t _Int, typename _Tp, size_t _Nm>
> > > > + constexpr _Tp&
> > > > + get(array<_Tp, _Nm>&) noexcept;
> > > > +
> > > > + template<size_t _Int, typename _Tp, size_t _Nm>
> > > > + constexpr _Tp&&
> > > > + get(array<_Tp, _Nm>&&) noexcept;
> > > > +
> > > > + template<size_t _Int, typename _Tp, size_t _Nm>
> > > > + constexpr const _Tp&
> > > > + get(const array<_Tp, _Nm>&) noexcept;
> > > > +
> > > > + template<size_t _Int, typename _Tp, size_t _Nm>
> > > > + 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<typename _Tp>
> > > > + inline constexpr bool __is_tuple_v = false;
> > > > +
> > > > + template<typename... _Ts>
> > > > + inline constexpr bool __is_tuple_v<tuple<_Ts...>> = true;
> > > > +
> > > > + // TODO: Reuse __is_tuple_like from <type_traits>?
> > > > + template<typename _Tp>
> > > > + inline constexpr bool __is_tuple_like_v = false;
> > > > +
> > > > + template<typename... _Elements>
> > > > + inline constexpr bool __is_tuple_like_v<tuple<_Elements...>> =
> > > > true;
> > > > +
> > > > + template<typename _T1, typename _T2>
> > > > + inline constexpr bool __is_tuple_like_v<pair<_T1, _T2>> = true;
> > > > +
> > > > + template<typename _Tp, size_t _Nm>
> > > > + inline constexpr bool __is_tuple_like_v<array<_Tp, _Nm>> = true;
> > > > +
> > > > + // __is_tuple_like_v<subrange> is defined in <bits/ranges_util.h>.
> > > > +
> > > > + template<typename _Tp>
> > > > + concept __tuple_like = __is_tuple_like_v<remove_cvref_t<_Tp>>;
> > > > +
> > > > + template<typename _Tp>
> > > > + concept __pair_like = __tuple_like<_Tp> &&
> > > > tuple_size_v<remove_cvref_t<_Tp>> == 2;
> > > > +
> > > > + template<typename _Tp, typename _Tuple>
> > > > + concept __eligible_tuple_like
> > > > + = __detail::__different_from<_Tp, _Tuple> && __tuple_like<_Tp>
> > > > + && (tuple_size_v<remove_cvref_t<_Tp>> == tuple_size_v<_Tuple>)
> > > > + && !ranges::__detail::__is_subrange<remove_cvref_t<_Tp>>;
> > > > +
> > > > + template<typename _Tp, typename _Pair>
> > > > + concept __eligible_pair_like
> > > > + = __detail::__different_from<_Tp, _Pair> && __pair_like<_Tp>
> > > > + && !ranges::__detail::__is_subrange<remove_cvref_t<_Tp>>;
> > > > +#endif // C++23
> > > > +
> > > > template<typename _U1, typename _U2> 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<typename _UPair>
> > > > + static constexpr bool
> > > > + _S_constructible_from_pair_like()
> > > > + {
> > > > + return
> > > > _S_constructible<decltype(std::get<0>(std::declval<_UPair>())),
> > > > +
> > > > decltype(std::get<1>(std::declval<_UPair>()))>();
> > > > + }
> > > > +
> > > > + template<typename _UPair>
> > > > + static constexpr bool
> > > > + _S_convertible_from_pair_like()
> > > > + {
> > > > + return
> > > > _S_convertible<decltype(std::get<0>(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<pair> _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.
>
> Huh, oops. Does an explicit ctor participate in overload resolution
> and then get checked if it's usable, or is it remove from overload
> resolution earlier?
> It looks like I decided the answer was the latter for pair and the
> former for tuple.
AFAICT if we know the explicitness of the ctor early (either because the
ctor is a non-template or it has a non-dependent explicit-spec), then we
remove it from the overload set early, in which case it'd be useful to
give the deleted ctor the right explicit-spec to avoid unecessary
constraint checking etc.
Otherwise, for ctor templates with a dependent explicit-spec such as
these tuple/pair ones, we must wait until after deduction to check
explicitness which means constraints are checked first. And by then
we already know that __dangles is true, so we presumably want overload
resolution to fail regardless. Whether that's due to selecting a
(viable) deleted non-explicit ctor (if we omit the explicit-spec) or due
to there being no viable non-explicit ctor (if we carry over the
explicit-spec) shouldn't make a difference, I think?
So it seems unnecessary to give these deleted overloads an
explicit-spec; it wouldn't be considered unless overload resolution
is destined to fail anyway.
I'm not totally confident about this assessment though so I'll
just carry over the explicit-spec for now.
>
> >
> > >
> > >
> > >
> > > > 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 <bits/version.h>
> > > >
> > > > 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<typename _Cat, typename _Tp, typename _Up>
> > > > + constexpr _Cat
> > > > + __tuple_cmp(const _Tp&, const _Up&, index_sequence<>);
> > > > +
> > > > + template<typename _Cat, typename _Tp, typename _Up,
> > > > + 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<typename _UTuple, size_t... _Is>
> > > > + 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<typename _Alloc>
> > > > _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<typename _Alloc, typename _UTuple, size_t... _Is>
> > > > + 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<typename... _UElements>
> > > > _GLIBCXX20_CONSTEXPR
> > > > void
> > > > @@ -470,6 +503,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > > }
> > > > #endif // C++23
> > > >
> > > > +#if __cpp_lib_tuple_like // >= C++23
> > > > + template<typename _UTuple>
> > > > + 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<typename _UTuple>
> > > > + 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<typename _UTuple>
> > > > + 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<typename _Alloc>
> > > > _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<typename _Alloc, typename _UTuple>
> > > > + 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<typename _UHead>
> > > > _GLIBCXX20_CONSTEXPR
> > > > void
> > > > @@ -667,6 +735,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > > }
> > > > #endif // C++23
> > > >
> > > > +#if __cpp_lib_tuple_like // >= C++23
> > > > + template<typename _UTuple>
> > > > + constexpr void
> > > > + _M_assign(__tuple_like_tag_t, _UTuple&& __u)
> > > > + { _M_head(*this) = std::get<_Idx>(std::forward<_UTuple>(__u)); }
> > > > +
> > > > + template<typename _UTuple>
> > > > + 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<typename _UTuple>
> > > > + static consteval bool
> > > > + __constructible_from_tuple_like()
> > > > + {
> > > > + return []<size_t... _Is>(index_sequence<_Is...>) {
> > > > + return
> > > > __constructible<decltype(std::get<_Is>(std::declval<_UTuple>()))...>();
> > > > + }(make_index_sequence<sizeof...(_Elements)>{});
> > > > + }
> > > > +
> > > > + template<typename _UTuple>
> > > > + static consteval bool
> > > > + __convertible_from_tuple_like()
> > > > + {
> > > > + return []<size_t... _Is>(index_sequence<_Is...>) {
> > > > + return
> > > > __convertible<decltype(std::get<_Is>(std::declval<_UTuple>()))...>();
> > > > + }(make_index_sequence<sizeof...(_Elements)>{});
> > >
> > > 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<tuple> _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<sizeof...(_Elements)>{})
> > > > + { }
> > > > #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<typename _Alloc, __tuple_like _UTuple>
> > > > - constexpr explicit(...)
> > > > - tuple(allocator_arg_t __tag, const _Alloc& __a, _UTuple&& __u);
> > > > +#if __cpp_lib_tuple_like // >= C++23
> > > > + template<typename _Alloc, __eligible_tuple_like<tuple> _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<sizeof...(_Elements)>{})
> > > > + { }
> > > > #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?
>
> Yes, I think so. I can't see why we would want to permit a dangling
> reference there.
>
> e.g.
> std::array<long, 1> a{};
> std::tuple<const int&> t(a);
Sounds good.
>
>
