* [PATCH] libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...]
@ 2021-04-27 16:57 Patrick Palka
2021-04-27 21:50 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2021-04-27 16:57 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
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(-)
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...]
2021-04-27 16:57 [PATCH] libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...] Patrick Palka
@ 2021-04-27 21:50 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2021-04-27 21:50 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, libstdc++
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
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-04-27 21:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27 16:57 [PATCH] libstdc++: Fix various bugs in ranges_algo.h [PR100187, ...] Patrick Palka
2021-04-27 21:50 ` Jonathan Wakely
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).