<algorithm>: ranges::minmax should dereference iterators only once

stl/inc/algorithm

@@ -10102,9 +10102,20 @@ namespace ranges {
             if constexpr (forward_range<_Rng> && _Prefer_iterator_copies<iterator_t<_Rng>>) {
                 const auto _Found = _RANGES _Minmax_element_unchecked(
                     _STD move(_UFirst), _STD move(_ULast), _Pass_fn(_Pred), _Pass_fn(_Proj));
+                // Avoid repeatedly initializing objects from the result of an iterator dereference when doing so might
+                // not be idempotent. The `if constexpr` to avoid the extra branch in cases where it's not needed
+                if constexpr (!same_as<remove_cvref_t<range_reference_t<_Rng>>, _Vty>
+                              || is_rvalue_reference_v<range_reference_t<_Rng>>) {
+                    if (_Found.min == _Found.max) {
+                        // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example
+                        minmax_result<_Vty> _Result = {static_cast<_Vty>(*_Found.min), _Result.min};
+                        return _Result;
+                    }
+                }
                 return {static_cast<_Vty>(*_Found.min), static_cast<_Vty>(*_Found.max)};
             } else {
-                minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), static_cast<_Vty>(*_UFirst)};
+                // This initialization is correct, similar to WG21-N4928 [dcl.init.aggr]/6 example
+                minmax_result<_Vty> _Found = {static_cast<_Vty>(*_UFirst), _Found.min};
                 if (_UFirst == _ULast) {
                     return _Found;
                 }

tests/libcxx/expected_results.txt

@@ -918,7 +918,6 @@ std/algorithms/alg.sorting/alg.heap.operations/push.heap/ranges_push_heap.pass.c
 std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp FAIL
 std/algorithms/alg.sorting/alg.merge/ranges_inplace_merge.pass.cpp FAIL
 std/algorithms/alg.sorting/alg.merge/ranges_merge.pass.cpp FAIL
-std/algorithms/alg.sorting/alg.min.max/ranges.minmax.pass.cpp FAIL
 std/algorithms/alg.sorting/alg.min.max/requires_forward_iterator.fail.cpp FAIL
 std/algorithms/alg.sorting/alg.nth.element/ranges_nth_element.pass.cpp FAIL
 std/algorithms/alg.sorting/alg.partitions/ranges.is_partitioned.pass.cpp:0 FAIL

tests/libcxx/skipped_tests.txt

@@ -918,7 +918,6 @@ algorithms\alg.sorting\alg.heap.operations\push.heap\ranges_push_heap.pass.cpp
 algorithms\alg.sorting\alg.heap.operations\sort.heap\ranges_sort_heap.pass.cpp
 algorithms\alg.sorting\alg.merge\ranges_inplace_merge.pass.cpp
 algorithms\alg.sorting\alg.merge\ranges_merge.pass.cpp
-algorithms\alg.sorting\alg.min.max\ranges.minmax.pass.cpp
 algorithms\alg.sorting\alg.min.max\requires_forward_iterator.fail.cpp
 algorithms\alg.sorting\alg.nth.element\ranges_nth_element.pass.cpp
 algorithms\alg.sorting\alg.partitions\ranges.is_partitioned.pass.cpp

tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp

@@ -9,10 +9,12 @@
 #include <cassert>
 #include <concepts>
 #include <functional>
+#include <memory>
 #include <ranges>
 #include <span>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include <range_algorithm_support.hpp>
 
@@ -361,6 +363,64 @@ void test_gh_1893() {
     ASSERT(val == "meow");
 }
 
+class input_move_iterator {
+public:
+    using iterator_category = input_iterator_tag;
+    using iterator_concept  = input_iterator_tag;
+    using difference_type   = ptrdiff_t;
+    using value_type        = shared_ptr<int>;
+    using pointer           = shared_ptr<int>*;
+    using reference         = shared_ptr<int>&&;
+
+    input_move_iterator() = default;
+    explicit input_move_iterator(shared_ptr<int>* ptr) : m_ptr(ptr) {}
+
+    reference operator*() const {
+        return ranges::iter_move(m_ptr);
+    }
+    pointer operator->() const {
+        return m_ptr;
+    }
+
+    input_move_iterator& operator++() {
+        ++m_ptr;
+        return *this;
+    }
+    input_move_iterator operator++(int) {
+        input_move_iterator tmp = *this;
+        ++*this;
+        return tmp;
+    }
+
+    friend bool operator==(const input_move_iterator& a, const input_move_iterator& b) = default;
+
+private:
+    shared_ptr<int>* m_ptr{nullptr};
+};
+
+void test_gh_2900() {
+    // GH-2900: <algorithm>: ranges::minmax initializes minmax_result with the moved value
+    {
+        // check that the random access iterator isn't moved from multiple times
+        const string str{"this long string will be dynamically allocated"};
+        vector<string> v{str};
+        ranges::subrange rng{move_iterator{v.begin()}, move_iterator{v.end()}};
+        auto result = ranges::minmax(rng);
+        assert(result.min == str);
+        assert(result.max == str);
+    }
+    {
+        // check that the input iterator isn't moved from multiple times
+        shared_ptr<int> a[] = {make_shared<int>(42)};
+        ranges::subrange rng{input_move_iterator{a}, input_move_iterator{a + 1}};
+        auto result = ranges::minmax(rng);
+        assert(a[0] == nullptr);
+        assert(result.min != nullptr);
+        assert(result.max == result.min);
+        assert(*result.max == 42);
+    }
+}
+
 int main() {
     STATIC_ASSERT((nonrange_tests(), true));
     nonrange_tests();
@@ -378,4 +438,5 @@ int main() {
     test_in<mm, const P>();
 
     test_gh_1893();
+    test_gh_2900();
 }