Fix excessive comparisons in minmax algorithms

stl/inc/algorithm

@@ -11112,35 +11112,43 @@ constexpr pair<_FwdIt, _FwdIt> _Minmax_element_unchecked(_FwdIt _First, _FwdIt _
     // find smallest and largest elements
     pair<_FwdIt, _FwdIt> _Found(_First, _First);
 
-    if (_First != _Last) {
-        while (++_First != _Last) { // process one or two elements
-            _FwdIt _Next = _First;
-            if (++_Next == _Last) { // process last element
+    if (_First == _Last || ++_First == _Last) {
+        return _Found;
+    }
+
+    if (_DEBUG_LT_PRED(_Pred, *_First, *_Found.first)) {
+        _Found.first = _First;
+    } else {
+        _Found.second = _First;
+    }
+
+    while (++_First != _Last) { // process one or two elements
+        _FwdIt _Next = _First;
+        if (++_Next == _Last) { // process last element
+            if (_DEBUG_LT_PRED(_Pred, *_First, *_Found.first)) {
+                _Found.first = _First;
+            } else if (!_DEBUG_LT_PRED(_Pred, *_First, *_Found.second)) {
+                _Found.second = _First;
+            }
+        } else { // process next two elements
+            if (_DEBUG_LT_PRED(_Pred, *_Next, *_First)) { // test _Next for new smallest
+                if (_DEBUG_LT_PRED(_Pred, *_Next, *_Found.first)) {
+                    _Found.first = _Next;
+                }
+
+                if (!_DEBUG_LT_PRED(_Pred, *_First, *_Found.second)) {
+                    _Found.second = _First;
+                }
+            } else { // test _First for new smallest
                 if (_DEBUG_LT_PRED(_Pred, *_First, *_Found.first)) {
                     _Found.first = _First;
-                } else if (!_DEBUG_LT_PRED(_Pred, *_First, *_Found.second)) {
-                    _Found.second = _First;
                 }
-            } else { // process next two elements
-                if (_DEBUG_LT_PRED(_Pred, *_Next, *_First)) { // test _Next for new smallest
-                    if (_DEBUG_LT_PRED(_Pred, *_Next, *_Found.first)) {
-                        _Found.first = _Next;
-                    }
 
-                    if (!_DEBUG_LT_PRED(_Pred, *_First, *_Found.second)) {
-                        _Found.second = _First;
-                    }
-                } else { // test _First for new smallest
-                    if (_DEBUG_LT_PRED(_Pred, *_First, *_Found.first)) {
-                        _Found.first = _First;
-                    }
-
-                    if (!_DEBUG_LT_PRED(_Pred, *_Next, *_Found.second)) {
-                        _Found.second = _Next;
-                    }
+                if (!_DEBUG_LT_PRED(_Pred, *_Next, *_Found.second)) {
+                    _Found.second = _Next;
                 }
-                _First = _Next;
             }
+            _First = _Next;
         }
     }
 
@@ -11239,10 +11247,16 @@ namespace ranges {
 
             min_max_result<_It> _Found{_First, _First};
 
-            if (_First == _Last) {
+            if (_First == _Last || ++_First == _Last) {
                 return _Found;
             }
 
+            if (_STD invoke(_Pred, _STD invoke(_Proj, *_First), _STD invoke(_Proj, *_Found.min))) {
+                _Found.min = _First;
+            } else {
+                _Found.max = _First;
+            }
+
             while (++_First != _Last) { // process one or two elements
                 _It _Prev = _First;
                 if (++_First == _Last) { // process last element
@@ -11431,10 +11445,16 @@ namespace ranges {
             } else {
                 // This initialization is correct, similar to the N4950 [dcl.init.aggr]/6 example
                 minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), _Found.min};
-                if (_UFirst == _ULast) {
+                if (++_UFirst == _ULast) {
                     return _Found;
                 }
 
+                if (_STD invoke(_Pred, _STD invoke(_Proj, *_UFirst), _STD invoke(_Proj, _Found.min))) {
+                    _Found.min = *_UFirst;
+                } else {
+                    _Found.max = *_UFirst;
+                }
+
                 while (++_UFirst != _ULast) { // process one or two elements
                     _Vty _Prev(*_UFirst);
                     if (++_UFirst == _ULast) { // process last element

tests/std/tests/Dev11_0532622_minmax_element/test.cpp

@@ -37,6 +37,18 @@ void test_all_permutations(vector<int>& v) {
     } while (next_permutation(v.begin(), v.end()));
 }
 
+// N5032 [alg.min.max]/32:
+// Complexity: Let N be last - first. At most max(floor(3/2 (N - 1)), 0) comparisons
+// and twice as many applications of the projection, if any.
+void test_cmp_count(initializer_list<int> v) {
+    size_t cmp_count = 0;
+    (void) minmax_element(v.begin(), v.end(), [&cmp_count](int left, int right) {
+        ++cmp_count;
+        return left < right;
+    });
+    assert(cmp_count <= 3 * (v.size() - 1) / 2);
+}
+
 int main() {
     vector<int> v;
 
@@ -92,4 +104,11 @@ int main() {
         assert(max_element(begin(data), end(data)) == begin(data) + 3);
         assert(minmax_element(begin(data), end(data)) == make_pair(begin(data) + 9, begin(data) + 16));
     }
+
+#if _ITERATOR_DEBUG_LEVEL < 2 // TRANSITION, GH-1006
+    test_cmp_count({1, 2, 3});
+    test_cmp_count({1, 2, 3, 4});
+    test_cmp_count({3, 2, 1});
+    test_cmp_count({4, 3, 2, 1});
+#endif // _ITERATOR_DEBUG_LEVEL < 2
 }

tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp

@@ -339,6 +339,89 @@ constexpr void mm_constexpr_tests() {
         ProxyRef::no>>();
 }
 
+template <class T>
+struct InputRange {
+    T* ptr;
+    size_t size;
+
+    struct iterator {
+        using value_type      = T;
+        using difference_type = ptrdiff_t;
+
+        T* ptr;
+
+        constexpr T operator*() const {
+            return *ptr;
+        }
+
+        constexpr iterator& operator++() {
+            ++ptr;
+            return *this;
+        }
+
+        constexpr void operator++(int) {
+            ++*this;
+        }
+
+        constexpr friend bool operator==(iterator left, iterator right) {
+            return left.ptr == right.ptr;
+        }
+    };
+
+    constexpr iterator begin() const {
+        return {ptr};
+    }
+
+    constexpr iterator end() const {
+        return {ptr + size};
+    }
+};
+
+// For minmax:
+// N5032 [alg.min.max]/23:
+// Complexity: At most (3/2) ranges::distance(r) applications of the corresponding predicate
+// and twice as many applications of the projection, if any.
+// For minmax_element:
+// N5032 [alg.min.max]/32:
+// Complexity: Let N be last - first. At most max(floor(3/2 (N - 1)), 0) comparisons
+// and twice as many applications of the projection, if any.
+constexpr void test_cmp_count(initializer_list<int> v) {
+    {
+        size_t cmp_count = 0;
+        (void) ranges::minmax(v, [&cmp_count](int left, int right) {
+            ++cmp_count;
+            return left < right;
+        });
+        ASSERT(cmp_count <= 3 * v.size() / 2);
+    }
+
+    {
+        InputRange r     = {v.data(), v.size()};
+        size_t cmp_count = 0;
+        (void) ranges::minmax(r, [&cmp_count](int left, int right) {
+            ++cmp_count;
+            return left < right;
+        });
+        ASSERT(cmp_count <= 3 * v.size() / 2);
+    }
+
+    {
+        size_t cmp_count = 0;
+        (void) ranges::minmax_element(v, [&cmp_count](int left, int right) {
+            ++cmp_count;
+            return left < right;
+        });
+        ASSERT(cmp_count <= 3 * (v.size() - 1) / 2);
+    }
+}
+
+constexpr void cmp_count_tests() {
+    test_cmp_count({1, 2, 3});
+    test_cmp_count({1, 2, 3, 4});
+    test_cmp_count({3, 2, 1});
+    test_cmp_count({4, 3, 2, 1});
+}
+
 void test_gh_1893() {
     // GH-1893: ranges::clamp was sometimes performing too many projections,
     // and we should conform at least in release mode.
@@ -437,6 +520,9 @@ int main() {
     static_assert((mm_constexpr_tests(), true));
     test_in<mm, const P>();
 
+    static_assert((cmp_count_tests(), true));
+    cmp_count_tests();
+
     test_gh_1893();
     test_gh_2900();
 }