From: Jonathan Wakely <jwakely@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc Patches <gcc-patches@gcc.gnu.org>,
"libstdc++" <libstdc++@gcc.gnu.org>
Subject: Re: [PATCH] libstdc++: Add missing std::move in ranges::copy/move/reverse_copy [PR101599]
Date: Mon, 26 Jul 2021 10:49:26 +0100 [thread overview]
Message-ID: <CACb0b4n_3VtKAOECuR3TdXy4zyVbr3Qjbd0-eMiwXJP+3xVqKQ@mail.gmail.com> (raw)
In-Reply-To: <20210723163448.3293782-1-ppalka@redhat.com>
On Fri, 23 Jul 2021 at 17:39, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> In passing, this also renames the template parameter _O2 to _Out2 in
> ranges::partition_copy and uglify its function parameters out_true
> and out_false.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk+branches?
Yes, thanks.
> PR libstdc++/101599
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/ranges_algo.h (__reverse_copy_fn::operator()):
> Add missing std::move in return statement.
> (__partition_copy_fn::operator()): Rename templtae parameter
> _O2 to _Out2. Uglify function parameters out_true and out_false.
> * include/bits/ranges_algobase.h (__copy_or_move): Add missing
> std::move to recursive call that unwraps a __normal_iterator
> output iterator.
> * testsuite/25_algorithms/copy/constrained.cc (test06): New test.
> * testsuite/25_algorithms/move/constrained.cc (test05): New test.
> ---
> libstdc++-v3/include/bits/ranges_algo.h | 20 +++++++++----------
> libstdc++-v3/include/bits/ranges_algobase.h | 2 +-
> .../25_algorithms/copy/constrained.cc | 13 ++++++++++++
> .../25_algorithms/move/constrained.cc | 13 ++++++++++++
> 4 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
> index 83371a4bdf0..8462521c369 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -1343,7 +1343,7 @@ namespace ranges
> *__result = *__tail;
> ++__result;
> }
> - return {__i, __result};
> + return {__i, std::move(__result)};
> }
>
> template<bidirectional_range _Range, weakly_incrementable _Out>
> @@ -2423,14 +2423,14 @@ namespace ranges
> struct __partition_copy_fn
> {
> template<input_iterator _Iter, sentinel_for<_Iter> _Sent,
> - weakly_incrementable _Out1, weakly_incrementable _O2,
> + weakly_incrementable _Out1, weakly_incrementable _Out2,
> typename _Proj = identity,
> indirect_unary_predicate<projected<_Iter, _Proj>> _Pred>
> requires indirectly_copyable<_Iter, _Out1>
> - && indirectly_copyable<_Iter, _O2>
> - constexpr partition_copy_result<_Iter, _Out1, _O2>
> + && indirectly_copyable<_Iter, _Out2>
> + constexpr partition_copy_result<_Iter, _Out1, _Out2>
> operator()(_Iter __first, _Sent __last,
> - _Out1 __out_true, _O2 __out_false,
> + _Out1 __out_true, _Out2 __out_false,
> _Pred __pred, _Proj __proj = {}) const
> {
> for (; __first != __last; ++__first)
> @@ -2450,18 +2450,18 @@ namespace ranges
> }
>
> template<input_range _Range, weakly_incrementable _Out1,
> - weakly_incrementable _O2,
> + weakly_incrementable _Out2,
> typename _Proj = identity,
> indirect_unary_predicate<projected<iterator_t<_Range>, _Proj>>
> _Pred>
> requires indirectly_copyable<iterator_t<_Range>, _Out1>
> - && indirectly_copyable<iterator_t<_Range>, _O2>
> - constexpr partition_copy_result<borrowed_iterator_t<_Range>, _Out1, _O2>
> - operator()(_Range&& __r, _Out1 out_true, _O2 out_false,
> + && indirectly_copyable<iterator_t<_Range>, _Out2>
> + constexpr partition_copy_result<borrowed_iterator_t<_Range>, _Out1, _Out2>
> + operator()(_Range&& __r, _Out1 __out_true, _Out2 __out_false,
> _Pred __pred, _Proj __proj = {}) const
> {
> return (*this)(ranges::begin(__r), ranges::end(__r),
> - std::move(out_true), std::move(out_false),
> + std::move(__out_true), std::move(__out_false),
> std::move(__pred), std::move(__proj));
> }
> };
> diff --git a/libstdc++-v3/include/bits/ranges_algobase.h b/libstdc++-v3/include/bits/ranges_algobase.h
> index c1037657c4c..78c295981d5 100644
> --- a/libstdc++-v3/include/bits/ranges_algobase.h
> +++ b/libstdc++-v3/include/bits/ranges_algobase.h
> @@ -244,7 +244,7 @@ namespace ranges
> else if constexpr (__is_normal_iterator<_Out>)
> {
> auto [__in,__out]
> - = ranges::__copy_or_move<_IsMove>(__first, __last, __result.base());
> + = ranges::__copy_or_move<_IsMove>(std::move(__first), __last, __result.base());
> return {std::move(__in), decltype(__result){__out}};
> }
> else if constexpr (sized_sentinel_for<_Sent, _Iter>)
> diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constrained.cc
> index 77ecf99d5b1..a05948a49c6 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/copy/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/copy/constrained.cc
> @@ -26,6 +26,7 @@
> using __gnu_test::test_container;
> using __gnu_test::test_range;
> using __gnu_test::input_iterator_wrapper;
> +using __gnu_test::input_iterator_wrapper_nocopy;
> using __gnu_test::output_iterator_wrapper;
> using __gnu_test::forward_iterator_wrapper;
>
> @@ -214,6 +215,17 @@ test05()
> return ok;
> }
>
> +void
> +test06()
> +{
> + // PR libstdc++/101599
> + int x[] = {1,2,3};
> + test_range<int, input_iterator_wrapper_nocopy> rx(x);
> + std::vector<int> v(4, 0);
> + ranges::copy(rx, v.begin());
> + VERIFY( ranges::equal(v, (int[]){1,2,3,0}) );
> +}
> +
> int
> main()
> {
> @@ -222,4 +234,5 @@ main()
> static_assert(test03());
> test04();
> static_assert(test05());
> + test06();
> }
> diff --git a/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> index 1cdfbdf23bc..2ce27b663a2 100644
> --- a/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> +++ b/libstdc++-v3/testsuite/25_algorithms/move/constrained.cc
> @@ -26,6 +26,7 @@
> using __gnu_test::test_container;
> using __gnu_test::test_range;
> using __gnu_test::input_iterator_wrapper;
> +using __gnu_test::input_iterator_wrapper_nocopy;
> using __gnu_test::output_iterator_wrapper;
> using __gnu_test::forward_iterator_wrapper;
>
> @@ -193,6 +194,17 @@ test04()
> VERIFY( ranges::count(y, 0, &X::moved) == 6 );
> }
>
> +void
> +test05()
> +{
> + // PR libstdc++/101599
> + int x[] = {1,2,3};
> + test_range<int, input_iterator_wrapper_nocopy> rx(x);
> + std::vector<int> v(4, 0);
> + ranges::move(rx, v.begin());
> + VERIFY( ranges::equal(v, (int[]){1,2,3,0}) );
> +}
> +
> int
> main()
> {
> @@ -200,4 +212,5 @@ main()
> test02();
> static_assert(test03());
> test04();
> + test05();
> }
> --
> 2.32.0.349.gdaab8a564f
>
prev parent reply other threads:[~2021-07-26 9:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-23 16:34 Patrick Palka
2021-07-26 9:49 ` Jonathan Wakely [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACb0b4n_3VtKAOECuR3TdXy4zyVbr3Qjbd0-eMiwXJP+3xVqKQ@mail.gmail.com \
--to=jwakely@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
--cc=ppalka@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).