On Wed, 3 Sept 2025 at 07:16, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> 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&&
Good idea. I think I should probably just extend the testcase to cover
every combination of lvalue/rvalue members, lvalue/rvalue pairs, and
const/non-const pairs.
>
> +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
>>
