diff mbox series

libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...]

Message ID 20210427165746.3019497-1-ppalka@redhat.com
State New
Headers show
Series libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...] | expand

Commit Message

Patrick Palka April 27, 2021, 4:57 p.m. UTC
This fixes some bugs with our ranges algorithms in uncommon situations,
such as when the return type of a predicate is a non-copyable class type
that's implicitly convertible to bool (PR100187), when a comparison
predicate isn't invocable as an rvalue (PR100237), and when the return
type of a projection function is non-copyable (PR100249).

This also fixes PR100287, which reports that we're moving __first twice
when constructing an empty subrange with it in ranges::partition.

Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
the releases branches?  I wasn't sure if it's worthwhile to include
tests for these bugs given their relatively obscure nature.

libstdc++-v3/ChangeLog:

	PR libstdc++/100187
	PR libstdc++/100237
	PR libstdc++/100249
	PR libstdc++/100287
	* include/bits/ranges_algo.h (__search_n_fn::operator()): Give
	__value_comp lambda an explicit bool return type.
	(__is_permutation_fn::operator()): Give __proj_scan local
	variable auto&& return type.  Give __comp_scan lambda an
	explicit bool return type.
	(__remove_fn::operator()): Give __pred lambda an explicit
	bool return type.
	(__partition_fn::operator()): Don't std::move __first twice
	when returning an empty range.
	(__min_fn::operator()): Don't std::move __comp.
	(__max_fn::operator()): Likewise.
	(__minmax_fn::operator()): Likewise.
---
 libstdc++-v3/include/bits/ranges_algo.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Jonathan Wakely April 27, 2021, 9:50 p.m. UTC | #1
On Tue, 27 Apr 2021, 21:37 Patrick Palka via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> This fixes some bugs with our ranges algorithms in uncommon situations,
> such as when the return type of a predicate is a non-copyable class type
> that's implicitly convertible to bool (PR100187), when a comparison
> predicate isn't invocable as an rvalue (PR100237), and when the return
> type of a projection function is non-copyable (PR100249).
>
> This also fixes PR100287, which reports that we're moving __first twice
> when constructing an empty subrange with it in ranges::partition.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
> the releases branches?  I wasn't sure if it's worthwhile to include
> tests for these bugs given their relatively obscure nature.
>

OK for trunk.

I think they're also find for the branches, after some time to bake on
trunk. They might be obscure, but the changes are also safe and so there's
little downside to backporting them.




> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/100187
>         PR libstdc++/100237
>         PR libstdc++/100249
>         PR libstdc++/100287
>         * include/bits/ranges_algo.h (__search_n_fn::operator()): Give
>         __value_comp lambda an explicit bool return type.
>         (__is_permutation_fn::operator()): Give __proj_scan local
>         variable auto&& return type.  Give __comp_scan lambda an
>         explicit bool return type.
>         (__remove_fn::operator()): Give __pred lambda an explicit
>         bool return type.
>         (__partition_fn::operator()): Don't std::move __first twice
>         when returning an empty range.
>         (__min_fn::operator()): Don't std::move __comp.
>         (__max_fn::operator()): Likewise.
>         (__minmax_fn::operator()): Likewise.
> ---
>  libstdc++-v3/include/bits/ranges_algo.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_algo.h
> b/libstdc++-v3/include/bits/ranges_algo.h
> index 23e6480b9a8..cda3042c11f 100644
> --- a/libstdc++-v3/include/bits/ranges_algo.h
> +++ b/libstdc++-v3/include/bits/ranges_algo.h
> @@ -562,7 +562,7 @@ namespace ranges
>         if (__count <= 0)
>           return {__first, __first};
>
> -       auto __value_comp = [&] <typename _Rp> (_Rp&& __arg) {
> +       auto __value_comp = [&] <typename _Rp> (_Rp&& __arg) -> bool {
>             return std::__invoke(__pred, std::forward<_Rp>(__arg),
> __value);
>         };
>         if (__count == 1)
> @@ -805,8 +805,8 @@ namespace ranges
>
>         for (auto __scan = __first1; __scan != __last1; ++__scan)
>           {
> -           auto __proj_scan = std::__invoke(__proj1, *__scan);
> -           auto __comp_scan = [&] <typename _Tp> (_Tp&& __arg) {
> +           auto&& __proj_scan = std::__invoke(__proj1, *__scan);
> +           auto __comp_scan = [&] <typename _Tp> (_Tp&& __arg) -> bool {
>               return std::__invoke(__pred, __proj_scan,
>                                    std::forward<_Tp>(__arg));
>             };
> @@ -1256,7 +1256,7 @@ namespace ranges
>        operator()(_Iter __first, _Sent __last,
>                  const _Tp& __value, _Proj __proj = {}) const
>        {
> -       auto __pred = [&] (auto&& __arg) {
> +       auto __pred = [&] (auto&& __arg) -> bool {
>           return std::forward<decltype(__arg)>(__arg) == __value;
>         };
>         return ranges::remove_if(__first, __last,
> @@ -2537,11 +2537,11 @@ namespace ranges
>         else
>           {
>             if (__first == __last)
> -             return {std::move(__first), std::move(__first)};
> +             return {__first, __first};
>
>             while (std::__invoke(__pred, std::__invoke(__proj, *__first)))
>               if (++__first == __last)
> -               return {std::move(__first), std::move(__first)};
> +               return {__first, __first};
>
>             auto __next = __first;
>             while (++__next != __last)
> @@ -3118,7 +3118,7 @@ namespace ranges
>        operator()(const _Tp& __a, const _Tp& __b,
>                  _Comp __comp = {}, _Proj __proj = {}) const
>        {
> -       if (std::__invoke(std::move(__comp),
> +       if (std::__invoke(__comp,
>                           std::__invoke(__proj, __b),
>                           std::__invoke(__proj, __a)))
>           return __b;
> @@ -3172,7 +3172,7 @@ namespace ranges
>        operator()(const _Tp& __a, const _Tp& __b,
>                  _Comp __comp = {}, _Proj __proj = {}) const
>        {
> -       if (std::__invoke(std::move(__comp),
> +       if (std::__invoke(__comp,
>                           std::__invoke(__proj, __a),
>                           std::__invoke(__proj, __b)))
>           return __b;
> @@ -3272,7 +3272,7 @@ namespace ranges
>        operator()(const _Tp& __a, const _Tp& __b,
>                  _Comp __comp = {}, _Proj __proj = {}) const
>        {
> -       if (std::__invoke(std::move(__comp),
> +       if (std::__invoke(__comp,
>                           std::__invoke(__proj, __b),
>                           std::__invoke(__proj, __a)))
>           return {__b, __a};
> --
> 2.31.1.362.g311531c9de
>
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
index 23e6480b9a8..cda3042c11f 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -562,7 +562,7 @@  namespace ranges
 	if (__count <= 0)
 	  return {__first, __first};
 
-	auto __value_comp = [&] <typename _Rp> (_Rp&& __arg) {
+	auto __value_comp = [&] <typename _Rp> (_Rp&& __arg) -> bool {
 	    return std::__invoke(__pred, std::forward<_Rp>(__arg), __value);
 	};
 	if (__count == 1)
@@ -805,8 +805,8 @@  namespace ranges
 
 	for (auto __scan = __first1; __scan != __last1; ++__scan)
 	  {
-	    auto __proj_scan = std::__invoke(__proj1, *__scan);
-	    auto __comp_scan = [&] <typename _Tp> (_Tp&& __arg) {
+	    auto&& __proj_scan = std::__invoke(__proj1, *__scan);
+	    auto __comp_scan = [&] <typename _Tp> (_Tp&& __arg) -> bool {
 	      return std::__invoke(__pred, __proj_scan,
 				   std::forward<_Tp>(__arg));
 	    };
@@ -1256,7 +1256,7 @@  namespace ranges
       operator()(_Iter __first, _Sent __last,
 		 const _Tp& __value, _Proj __proj = {}) const
       {
-	auto __pred = [&] (auto&& __arg) {
+	auto __pred = [&] (auto&& __arg) -> bool {
 	  return std::forward<decltype(__arg)>(__arg) == __value;
 	};
 	return ranges::remove_if(__first, __last,
@@ -2537,11 +2537,11 @@  namespace ranges
 	else
 	  {
 	    if (__first == __last)
-	      return {std::move(__first), std::move(__first)};
+	      return {__first, __first};
 
 	    while (std::__invoke(__pred, std::__invoke(__proj, *__first)))
 	      if (++__first == __last)
-		return {std::move(__first), std::move(__first)};
+		return {__first, __first};
 
 	    auto __next = __first;
 	    while (++__next != __last)
@@ -3118,7 +3118,7 @@  namespace ranges
       operator()(const _Tp& __a, const _Tp& __b,
 		 _Comp __comp = {}, _Proj __proj = {}) const
       {
-	if (std::__invoke(std::move(__comp),
+	if (std::__invoke(__comp,
 			  std::__invoke(__proj, __b),
 			  std::__invoke(__proj, __a)))
 	  return __b;
@@ -3172,7 +3172,7 @@  namespace ranges
       operator()(const _Tp& __a, const _Tp& __b,
 		 _Comp __comp = {}, _Proj __proj = {}) const
       {
-	if (std::__invoke(std::move(__comp),
+	if (std::__invoke(__comp,
 			  std::__invoke(__proj, __a),
 			  std::__invoke(__proj, __b)))
 	  return __b;
@@ -3272,7 +3272,7 @@  namespace ranges
       operator()(const _Tp& __a, const _Tp& __b,
 		 _Comp __comp = {}, _Proj __proj = {}) const
       {
-	if (std::__invoke(std::move(__comp),
+	if (std::__invoke(__comp,
 			  std::__invoke(__proj, __b),
 			  std::__invoke(__proj, __a)))
 	  return {__b, __a};