On Mon, 15 Jan 2024 at 16:51, Jonathan Wakely <jwak...@redhat.com> wrote: > > On Mon, 15 Jan 2024 at 16:27, Patrick Palka <ppa...@redhat.com> wrote: > > > > On Sat, 13 Jan 2024, Jonathan Wakely wrote: > > > > > On Sat, 13 Jan 2024 at 00:06, Patrick Palka <ppa...@redhat.com> wrote: > > > > > > > > On Fri, 12 Jan 2024, Jonathan Wakely wrote: > > > > > > > > > On Fri, 12 Jan 2024 at 18:33, Patrick Palka <ppa...@redhat.com> wrote: > > > > > > > > > > > > On Fri, 12 Jan 2024, Jonathan Wakely wrote: > > > > > > > > > > > > > On Fri, 12 Jan 2024 at 17:55, Patrick Palka <ppa...@redhat.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > On Thu, 11 Jan 2024, Jonathan Wakely wrote: > > > > > > > > > > > > > > > > > I'd like to commit this to trunk for GCC 14. Please take a > > > > > > > > > look. > > > > > > > > > > > > > > > > > > -- >8 -- > > > > > > > > > > > > > > > > > > This is the last part of PR libstdc++/108822 implementing > > > > > > > > > P2255R2, which > > > > > > > > > makes it ill-formed to create a std::tuple that would bind a > > > > > > > > > reference > > > > > > > > > to a temporary. > > > > > > > > > > > > > > > > > > The dangling checks are implemented as deleted constructors > > > > > > > > > for C++20 > > > > > > > > > and higher, and as Debug Mode static assertions in the > > > > > > > > > constructor body > > > > > > > > > for older standards. This is similar to the > > > > > > > > > r13-6084-g916ce577ad109b > > > > > > > > > changes for std::pair. > > > > > > > > > > > > > > > > > > As part of this change, I've reimplemented most of std::tuple > > > > > > > > > for C++20, > > > > > > > > > making use of concepts to replace the enable_if constraints, > > > > > > > > > and using > > > > > > > > > conditional explicit to avoid duplicating most constructors. > > > > > > > > > We could > > > > > > > > > use conditional explicit for the C++11 implementation too > > > > > > > > > (with pragmas > > > > > > > > > to disables the -Wc++17-extensions warnings), but that should > > > > > > > > > be done as > > > > > > > > > a stage 1 change for GCC 15 rather than now. > > > > > > > > > > > > > > > > > > The partial specialization for std::tuple<T1, T2> is no > > > > > > > > > longer used for > > > > > > > > > C++20 (or more precisely, for a C++20 compiler that supports > > > > > > > > > concepts > > > > > > > > > and conditional explicit). The additional constructors and > > > > > > > > > assignment > > > > > > > > > operators that take std::pair arguments have been added to > > > > > > > > > the C++20 > > > > > > > > > implementation of the primary template, with > > > > > > > > > sizeof...(_Elements)==2 > > > > > > > > > constraints. This avoids reimplementing all the other > > > > > > > > > constructors in > > > > > > > > > the std::tuple<T1, T2> partial specialization to use > > > > > > > > > concepts. This way > > > > > > > > > we avoid four implementations of every constructor and only > > > > > > > > > have three! > > > > > > > > > (The primary template has an implementation of each > > > > > > > > > constructor for > > > > > > > > > C++11 and another for C++20, and the tuple<T1,T2> > > > > > > > > > specialization has an > > > > > > > > > implementation of each for C++11, so that's three for each > > > > > > > > > constructor.) > > > > > > > > > > > > > > > > > > In order to make the constraints more efficient on the C++20 > > > > > > > > > version of > > > > > > > > > the default constructor I've also added a variable template > > > > > > > > > for the > > > > > > > > > __is_implicitly_default_constructible trait, implemented > > > > > > > > > using concepts. > > > > > > > > > > > > > > > > > > libstdc++-v3/ChangeLog: > > > > > > > > > > > > > > > > > > PR libstdc++/108822 > > > > > > > > > * include/std/tuple (tuple): Add checks for dangling > > > > > > > > > references. > > > > > > > > > Reimplement constraints and constant expressions using > > > > > > > > > C++20 > > > > > > > > > features. > > > > > > > > > * include/std/type_traits [C++20] > > > > > > > > > (__is_implicitly_default_constructible_v): Define. > > > > > > > > > (__is_implicitly_default_constructible): Use variable > > > > > > > > > template. > > > > > > > > > * testsuite/20_util/tuple/dangling_ref.cc: New test. > > > > > > > > > --- > > > > > > > > > libstdc++-v3/include/std/tuple | 1021 > > > > > > > > > ++++++++++++----- > > > > > > > > > libstdc++-v3/include/std/type_traits | 11 + > > > > > > > > > .../testsuite/20_util/tuple/dangling_ref.cc | 105 ++ > > > > > > > > > 3 files changed, 841 insertions(+), 296 deletions(-) > > > > > > > > > create mode 100644 > > > > > > > > > libstdc++-v3/testsuite/20_util/tuple/dangling_ref.cc > > > > > > > > > > > > > > > > > > diff --git a/libstdc++-v3/include/std/tuple > > > > > > > > > b/libstdc++-v3/include/std/tuple > > > > > > > > > index 50e11843757..cd05b638923 100644 > > > > > > > > > --- a/libstdc++-v3/include/std/tuple > > > > > > > > > +++ b/libstdc++-v3/include/std/tuple > > > > > > > > > @@ -752,11 +752,467 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > > > > > template<typename... _Elements> > > > > > > > > > class tuple : public _Tuple_impl<0, _Elements...> > > > > > > > > > { > > > > > > > > > - typedef _Tuple_impl<0, _Elements...> _Inherited; > > > > > > > > > + using _Inherited = _Tuple_impl<0, _Elements...>; > > > > > > > > > > > > > > > > > > template<bool _Cond> > > > > > > > > > using _TCC = _TupleConstraints<_Cond, _Elements...>; > > > > > > > > > > > > > > > > I guess this should be moved into the #else branch if it's not > > > > > > > > used in > > > > > > > > the new impl. > > > > > > > > > > > > > > Ah yes, I left them there until I was sure I wouldn't need them > > > > > > > ... > > > > > > > then didn't move them when I didn't need them. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#if __cpp_concepts && __cpp_conditional_explicit // >= C++20 > > > > > > > > > + template<typename... _UTypes> > > > > > > > > > + static consteval bool > > > > > > > > > + __constructible() > > > > > > > > > + { > > > > > > > > > + if constexpr (sizeof...(_UTypes) == > > > > > > > > > sizeof...(_Elements)) > > > > > > > > > + return (is_constructible_v<_Elements, _UTypes> && > > > > > > > > > ...); > > > > > > > > > > > > > > > > IIUC this (and all the other new constraints) won't > > > > > > > > short-circuit like > > > > > > > > the old versions do :/ Not sure how much that matters? > > > > > > > > > > > > > > Yeah, I thought about that, but we have efficient built-ins for > > > > > > > these > > > > > > > traits now, so I think it's probably OK? > > > > > > > > > > > > Performance wise agreed, though I suppose removing the short > > > > > > circuiting > > > > > > could break existing (though not necessarily valid) code that relied > > > > > > on it to prevent an ill-formed template instantiation. It seems > > > > > > the standard https://eel.is/c++draft/tuple uses conjunction_v in > > > > > > some > > > > > > constraints, and fold-expressions in others, implying short > > > > > > circuiting > > > > > > in some cases but not others? > > > > > > > > > > > > > > > > > > > > If not we could go back to sharing the _TupleConstraints > > > > > > > implementations. > > > > > > > > > > > > IMHO I'd be more comfortable with that. > > > > > > > > > > Here's an incremental diff to make that change: > > > > > > > > > > --- a/libstdc++-v3/include/std/tuple > > > > > +++ b/libstdc++-v3/include/std/tuple > > > > > @@ -763,7 +763,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __constructible() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_constructible_v<_Elements, _UTypes> && ...); > > > > > + return __and_v<is_constructible<_Elements, _UTypes>...>; > > > > > else > > > > > return false; > > > > > } > > > > > @@ -773,7 +773,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __nothrow_constructible() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_nothrow_constructible_v<_Elements, _UTypes> && > > > > > ...); > > > > > + return __and_v<is_nothrow_constructible<_Elements, > > > > > _UTypes>...>; > > > > > else > > > > > return false; > > > > > } > > > > > @@ -783,7 +783,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __convertible() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_convertible_v<_UTypes, _Elements> && ...); > > > > > + return __and_v<is_convertible<_UTypes, _Elements>...>; > > > > > else > > > > > return false; > > > > > } > > > > > @@ -1526,7 +1526,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __assignable() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_assignable_v<_Elements&, _UTypes> && ...); > > > > > + return __and_v<is_assignable<_Elements&, _UTypes>...>; > > > > > else > > > > > return false; > > > > > } > > > > > @@ -1536,7 +1536,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __nothrow_assignable() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_nothrow_assignable_v<_Elements&, _UTypes> && > > > > > ...); > > > > > + return __and_v<is_nothrow_assignable<_Elements&, > > > > > _UTypes>...>; > > > > > else > > > > > return false; > > > > > } > > > > > @@ -1547,7 +1547,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > > __const_assignable() > > > > > { > > > > > if constexpr (sizeof...(_UTypes) == sizeof...(_Elements)) > > > > > - return (is_assignable_v<const _Elements&, _UTypes> && > > > > > ...); > > > > > + return __and_v<is_assignable<const _Elements&, > > > > > _UTypes>...>; > > > > > else > > > > > return false; > > > > > } > > > > > > > > > > Happier with that? > > > > > > > > > > It passes all the tuple tests, I'm running the full suite now. > > > > > > > > > > > > > > > > > > LGTM > > > > > > Pushed to trunk - thanks for the reviews. > > > > I'm seeing a redefinition error when compiling <tuple> with > > -std=c++20 -U__cpp_conditional_explicit (which IIUC is intended > > to work?): > > Yes ... well, a compiler that doesn't define that is supposed to work. > Manually undef'ing predefined macros yourself is UB of course :-) > > > > /home/ppalka/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/tuple:1536:9: > > error: ‘template<class ... _Elements> template<class ... _UTypes> static > > consteval bool std::tuple< <template-parameter-1-1> > > >::__nothrow_assignable()’ cannot be overloaded with ‘template<class ... > > _Elements> template<class ... _UElements> static constexpr bool std::tuple< > > <template-parameter-1-1> >::__nothrow_assignable()’ > > 1536 | __nothrow_assignable() > > | ^~~~~~~~~~~~~~~~~~~~ > > /home/ppalka/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/tuple:1248:31: > > note: previous declaration ‘template<class ... _Elements> template<class > > ... _UElements> static constexpr bool std::tuple< <template-parameter-1-1> > > >::__nothrow_assignable()’ > > 1248 | static constexpr bool __nothrow_assignable() > > | ^~~~~~~~~~~~~~~~~~~~ > > > It needs this patch: > > --- a/libstdc++-v3/include/std/tuple > +++ b/libstdc++-v3/include/std/tuple > @@ -1237,20 +1237,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > _TCC<_Cond>::template __is_explicitly_constructible<_Args...>(), > bool>; > > - template<typename... _UElements> > - static constexpr > - __enable_if_t<sizeof...(_UElements) == sizeof...(_Elements), bool> > - __assignable() > - { return __and_<is_assignable<_Elements&, _UElements>...>::value; } > - > - // Condition for noexcept-specifier of an assignment operator. > - template<typename... _UElements> > - static constexpr bool __nothrow_assignable() > - { > - return > - __and_<is_nothrow_assignable<_Elements&, _UElements>...>::value; > - } > - > // Condition for noexcept-specifier of a constructor. > template<typename... _UElements> > static constexpr bool __nothrow_constructible() > @@ -1687,6 +1673,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > #else // ! concepts > > + private: > + template<typename... _UElements> > + static constexpr > + __enable_if_t<sizeof...(_UElements) == sizeof...(_Elements), bool> > + __assignable() > + { return __and_<is_assignable<_Elements&, _UElements>...>::value; } > + > + // Condition for noexcept-specifier of an assignment operator. > + template<typename... _UElements> > + static constexpr bool __nothrow_assignable() > + { > + return > + __and_<is_nothrow_assignable<_Elements&, _UElements>...>::value; > + } > + > + public: > + > _GLIBCXX20_CONSTEXPR > tuple& > operator=(__conditional_t<__assignable<const _Elements&...>(),
Pushed to trunk as r14-7256-g1e88a151f878e0 after testing on aarch64-linux. Thanks for noticing the bug.