| Message ID | 20241029131935.85441-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: Avoid unnecessary copies in ranges::min/max [PR112349] | expand |
On Tue, 29 Oct 2024 at 13:20, Patrick Palka <ppalka@redhat.com> wrote: > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps > 14? OK for both, thanks. > > -- >8 -- > > Use a local reference for the (now possibly lifetime extended) result of > *__first to avoid making unnecessary copies of it. > > PR libstdc++/112349 > > libstdc++-v3/ChangeLog: > > * include/bits/ranges_algo.h (__min_fn::operator()): Turn local > object __tmp into a reference. > * include/bits/ranges_util.h (__max_fn::operator()): Likewise. > * testsuite/25_algorithms/max/constrained.cc (test04): New test. > * testsuite/25_algorithms/min/constrained.cc (test04): New test. > --- > libstdc++-v3/include/bits/ranges_algo.h | 4 +-- > libstdc++-v3/include/bits/ranges_util.h | 4 +-- > .../25_algorithms/max/constrained.cc | 25 +++++++++++++++++++ > .../25_algorithms/min/constrained.cc | 25 +++++++++++++++++++ > 4 files changed, 54 insertions(+), 4 deletions(-) > > diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h > index bae36637b3e..e1aba256241 100644 > --- a/libstdc++-v3/include/bits/ranges_algo.h > +++ b/libstdc++-v3/include/bits/ranges_algo.h > @@ -2945,11 +2945,11 @@ namespace ranges > auto __result = *__first; > while (++__first != __last) > { > - auto __tmp = *__first; > + auto&& __tmp = *__first; > if (std::__invoke(__comp, > std::__invoke(__proj, __result), > std::__invoke(__proj, __tmp))) > - __result = std::move(__tmp); > + __result = std::forward<decltype(__tmp)>(__tmp); > } > return __result; > } > diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h > index 3f191e6d446..4a5349ae92a 100644 > --- a/libstdc++-v3/include/bits/ranges_util.h > +++ b/libstdc++-v3/include/bits/ranges_util.h > @@ -754,11 +754,11 @@ namespace ranges > auto __result = *__first; > while (++__first != __last) > { > - auto __tmp = *__first; > + auto&& __tmp = *__first; > if (std::__invoke(__comp, > std::__invoke(__proj, __tmp), > std::__invoke(__proj, __result))) > - __result = std::move(__tmp); > + __result = std::forward<decltype(__tmp)>(__tmp); > } > return __result; > } > diff --git a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc > index e7269e1b734..717900656bd 100644 > --- a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc > +++ b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc > @@ -73,10 +73,35 @@ test03() > VERIFY( ranges::max({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 4 ); > } > > +void > +test04() > +{ > + // PR libstdc++/112349 - ranges::max/min make unnecessary copies > + static int copies, moves; > + struct A { > + A(int m) : m(m) { } > + A(const A& other) : m(other.m) { ++copies; } > + A(A&& other) : m(other.m) { ++moves; } > + A& operator=(const A& other) { m = other.m; ++copies; return *this; } > + A& operator=(A&& other) { m = other.m; ++moves; return *this; } > + int m; > + }; > + A r[5] = {5, 4, 3, 2, 1}; > + ranges::max(r, std::less{}, &A::m); > + VERIFY( copies == 1 ); > + VERIFY( moves == 0 ); > + copies = moves = 0; > + A s[5] = {1, 2, 3, 4, 5}; > + ranges::max(s, std::less{}, &A::m); > + VERIFY( copies == 5 ); > + VERIFY( moves == 0 ); > +} > + > int > main() > { > test01(); > test02(); > test03(); > + test04(); > } > diff --git a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc > index 7198df69adf..d338a86f186 100644 > --- a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc > +++ b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc > @@ -73,10 +73,35 @@ test03() > VERIFY( ranges::min({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 1 ); > } > > +void > +test04() > +{ > + // PR libstdc++/112349 - ranges::max/min make unnecessary copies > + static int copies, moves; > + struct A { > + A(int m) : m(m) { } > + A(const A& other) : m(other.m) { ++copies; } > + A(A&& other) : m(other.m) { ++moves; } > + A& operator=(const A& other) { m = other.m; ++copies; return *this; } > + A& operator=(A&& other) { m = other.m; ++moves; return *this; } > + int m; > + }; > + A r[5] = {5, 4, 3, 2, 1}; > + ranges::min(r, std::less{}, &A::m); > + VERIFY( copies == 5 ); > + VERIFY( moves == 0 ); > + copies = moves = 0; > + A s[5] = {1, 2, 3, 4, 5}; > + ranges::min(s, std::less{}, &A::m); > + VERIFY( copies == 1 ); > + VERIFY( moves == 0 ); > +} > + > int > main() > { > test01(); > test02(); > test03(); > + test04(); > } > -- > 2.47.0.148.g6a11438f43 >
diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h index bae36637b3e..e1aba256241 100644 --- a/libstdc++-v3/include/bits/ranges_algo.h +++ b/libstdc++-v3/include/bits/ranges_algo.h @@ -2945,11 +2945,11 @@ namespace ranges auto __result = *__first; while (++__first != __last) { - auto __tmp = *__first; + auto&& __tmp = *__first; if (std::__invoke(__comp, std::__invoke(__proj, __result), std::__invoke(__proj, __tmp))) - __result = std::move(__tmp); + __result = std::forward<decltype(__tmp)>(__tmp); } return __result; } diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h index 3f191e6d446..4a5349ae92a 100644 --- a/libstdc++-v3/include/bits/ranges_util.h +++ b/libstdc++-v3/include/bits/ranges_util.h @@ -754,11 +754,11 @@ namespace ranges auto __result = *__first; while (++__first != __last) { - auto __tmp = *__first; + auto&& __tmp = *__first; if (std::__invoke(__comp, std::__invoke(__proj, __tmp), std::__invoke(__proj, __result))) - __result = std::move(__tmp); + __result = std::forward<decltype(__tmp)>(__tmp); } return __result; } diff --git a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc index e7269e1b734..717900656bd 100644 --- a/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc +++ b/libstdc++-v3/testsuite/25_algorithms/max/constrained.cc @@ -73,10 +73,35 @@ test03() VERIFY( ranges::max({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 4 ); } +void +test04() +{ + // PR libstdc++/112349 - ranges::max/min make unnecessary copies + static int copies, moves; + struct A { + A(int m) : m(m) { } + A(const A& other) : m(other.m) { ++copies; } + A(A&& other) : m(other.m) { ++moves; } + A& operator=(const A& other) { m = other.m; ++copies; return *this; } + A& operator=(A&& other) { m = other.m; ++moves; return *this; } + int m; + }; + A r[5] = {5, 4, 3, 2, 1}; + ranges::max(r, std::less{}, &A::m); + VERIFY( copies == 1 ); + VERIFY( moves == 0 ); + copies = moves = 0; + A s[5] = {1, 2, 3, 4, 5}; + ranges::max(s, std::less{}, &A::m); + VERIFY( copies == 5 ); + VERIFY( moves == 0 ); +} + int main() { test01(); test02(); test03(); + test04(); } diff --git a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc index 7198df69adf..d338a86f186 100644 --- a/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc +++ b/libstdc++-v3/testsuite/25_algorithms/min/constrained.cc @@ -73,10 +73,35 @@ test03() VERIFY( ranges::min({2,3,1,4}, ranges::greater{}, std::negate<>{}) == 1 ); } +void +test04() +{ + // PR libstdc++/112349 - ranges::max/min make unnecessary copies + static int copies, moves; + struct A { + A(int m) : m(m) { } + A(const A& other) : m(other.m) { ++copies; } + A(A&& other) : m(other.m) { ++moves; } + A& operator=(const A& other) { m = other.m; ++copies; return *this; } + A& operator=(A&& other) { m = other.m; ++moves; return *this; } + int m; + }; + A r[5] = {5, 4, 3, 2, 1}; + ranges::min(r, std::less{}, &A::m); + VERIFY( copies == 5 ); + VERIFY( moves == 0 ); + copies = moves = 0; + A s[5] = {1, 2, 3, 4, 5}; + ranges::min(s, std::less{}, &A::m); + VERIFY( copies == 1 ); + VERIFY( moves == 0 ); +} + int main() { test01(); test02(); test03(); + test04(); }