libstdc++: Avoid unnecessary copies in ranges::min/max [PR112349]

Use a local reference for the (now possibly lifetime extended) result of
*__first so that we copy it only when necessary.

	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.

Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
(cherry picked from commit b8314ebff2495ee22f9e2203093bdada9843a0f5)

diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
index 6347ec98606a068a7de8b85c83e7c0930fa3ab74..a29d387dc71142fab5ba84eaa5adf5fce4d7dba1 100644 (file)
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -2909,11 +2909,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 34d8b1e8f1432a8397f7096aaf52d7190fc7b6b5..aba5b6a88c0c8e2c9efed69520f4fc574acc7bd1 100644 (file)
--- a/libstdc++-v3/include/bits/ranges_util.h
+++ b/libstdc++-v3/include/bits/ranges_util.h
@@ -733,11 +733,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 e7269e1b734a8387ad148afd2ab33ea426fefe2f..ad2d47f2f101a1b8400cb22b3ea81ea723bd9df3 100644 (file)
--- 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, ranges::less{}, &A::m);
+  VERIFY( copies == 1 );
+  VERIFY( moves == 0 );
+  copies = moves = 0;
+  A s[5] = {1, 2, 3, 4, 5};
+  ranges::max(s, ranges::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 7198df69adf7bdcd25a3dd1217c0353ef80f6d1e..17048fda6394ae57bb6ec730c70af1acf3ffc840 100644 (file)
--- 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, ranges::less{}, &A::m);
+  VERIFY( copies == 5 );
+  VERIFY( moves == 0 );
+  copies = moves = 0;
+  A s[5] = {1, 2, 3, 4, 5};
+  ranges::min(s, ranges::less{}, &A::m);
+  VERIFY( copies == 1 );
+  VERIFY( moves == 0 );
+}
+
 int
 main()
 {
   test01();
   test02();
   test03();
+  test04();
 }