On Tue, Sep 2, 2025 at 9:46 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> Make the std::get<T> overloads for rvalues use std::forward<T>(p.first)
> not std::move(p.first), so that lvalue reference members are not
> incorrectly converted to rvalues.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/121745
> * include/bits/stl_pair.h (get): Use forward instead of move in
> std::get<T> overloads for rvalue pairs.
> * testsuite/20_util/pair/astuple/get_by_type.cc: Check rvalue
> arguments with reference members.
> ---
>
> Tested powerpc64-linux. We should backport this too.
>
LGTM, with additional test case suggested.
>
> libstdc++-v3/include/bits/stl_pair.h | 8 ++++----
> .../testsuite/20_util/pair/astuple/get_by_type.cc | 12 ++++++++++++
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/stl_pair.h
> b/libstdc++-v3/include/bits/stl_pair.h
> index 393f6a016196..661335b466a3 100644
> --- a/libstdc++-v3/include/bits/stl_pair.h
> +++ b/libstdc++-v3/include/bits/stl_pair.h
> @@ -1315,12 +1315,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template <typename _Tp, typename _Up>
> constexpr _Tp&&
> get(pair<_Tp, _Up>&& __p) noexcept
> - { return std::move(__p.first); }
> + { return std::forward<_Tp>(__p.first); }
>
> template <typename _Tp, typename _Up>
> constexpr const _Tp&&
> get(const pair<_Tp, _Up>&& __p) noexcept
> - { return std::move(__p.first); }
> + { return std::forward<const _Tp>(__p.first); }
>
> template <typename _Tp, typename _Up>
> constexpr _Tp&
> @@ -1335,12 +1335,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template <typename _Tp, typename _Up>
> constexpr _Tp&&
> get(pair<_Up, _Tp>&& __p) noexcept
> - { return std::move(__p.second); }
> + { return std::forward<_Tp>(__p.second); }
>
> template <typename _Tp, typename _Up>
> constexpr const _Tp&&
> get(const pair<_Up, _Tp>&& __p) noexcept
> - { return std::move(__p.second); }
> + { return std::forward<const _Tp>(__p.second); }
> #endif // __glibcxx_tuples_by_type
>
>
> diff --git a/libstdc++-v3/testsuite/20_util/pair/astuple/get_by_type.cc
> b/libstdc++-v3/testsuite/20_util/pair/astuple/get_by_type.cc
> index 33ebf7a46b90..9d934db0b69f 100644
> --- a/libstdc++-v3/testsuite/20_util/pair/astuple/get_by_type.cc
> +++ b/libstdc++-v3/testsuite/20_util/pair/astuple/get_by_type.cc
> @@ -33,3 +33,15 @@ void test01()
> const int&& cpsecond __attribute__((unused)) =
> std::get<int>(std::move(cp));
> }
> +
> +// PR libstdc++/121745 return of get(pair<_Up, _Tp>&& __p) may be
> ill-formed
> +void
> +test_pr121745(std::pair<float&, int&> p)
> +{
> + float& pfirst = std::get<float&>(std::move(p));
> + int& psecond = std::get<int&>(std::move(p));
> +
> + const auto& p2 = p;
> + float& p2first = std::get<float&>(std::move(p2));
> + int& p2second = std::get<int&>(std::move(p2));
> +}
>
I want to also see following test being added, that explains why we want to
use:
std::forward<_Tp>(__p.second);
Instead of, seemingly more obvious:
std::move(__p).second -> this will produce int& instead of int&&
+void
+test_pr121745(std::pair<float&&, int&&> p)
+{
+ float&& pfirst = std::get<float&&>(std::move(p));
+ int&& psecond = std::get<int&&>(std::move(p));
+
+ const auto& p2 = p;
+ float&& p2first = std::get<float&&>(std::move(p2));
+ int&& p2second = std::get<int&&>(std::move(p2));
+}
--
> --
> 2.51.0
>
>
