`<algorithm>`: `ranges::clamp`, the projection may be applied at most three times. (#1898)

* the projection may be applied at most three times.

Co-authored-by: Michael Schellenberger Costa <mschellenbergercosta@googlemail.com>
Co-authored-by: statementreply <statementreply@gmail.com>
Co-authored-by: Casey Carter <cacarter@microsoft.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>

stl/inc/algorithm

@@ -10221,11 +10221,13 @@ namespace ranges {
                 "The lower bound cannot be greater than the upper bound in a call to std::ranges::clamp "
                 "(N4861 [alg.clamp]/2).");
 
-            if (_STD invoke(_Pred, _STD invoke(_Proj, _Val), _STD invoke(_Proj, _Lo))) {
+            auto&& _Temp = _STD invoke(_Proj, _Val);
+            if (_STD invoke(_Pred, _STD forward<decltype(_Temp)>(_Temp), _STD invoke(_Proj, _Lo))) {
                 return _Lo;
             }
-
-            if (_STD invoke(_Pred, _STD invoke(_Proj, _Hi), _STD invoke(_Proj, _Val))) {
+            // The double forward is safe because regular_invocable requires that the invocation of the predicate not
+            // modify _Temp in a manner observable to equality-preserving expressions.
+            if (_STD invoke(_Pred, _STD invoke(_Proj, _Hi), _STD forward<decltype(_Temp)>(_Temp))) {
                 return _Hi;
             }
 

tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp

@@ -8,8 +8,10 @@
 #include <array>
 #include <cassert>
 #include <concepts>
+#include <functional>
 #include <ranges>
 #include <span>
+#include <string>
 #include <utility>
 
 #include <range_algorithm_support.hpp>
@@ -335,6 +337,30 @@ constexpr void mm_constexpr_tests() {
         ProxyRef::no>>();
 }
 
+void test_gh_1893() {
+    // ranges::clamp was sometimes performing too many projections,
+    // and we should conform at least in release mode.
+    // the test protects us from the wrong implementation with std::move instead of std::forward in ranges::clamp
+    // so reference_wrappers and the lambda are necessary.
+    string val           = "meow";
+    string low           = "m";
+    string high          = "n";
+    int projection_count = 0;
+    const auto clamped   = ranges::clamp(
+        ref(val), ref(low), ref(high), [](auto x, auto y) { return x < y; },
+        [&projection_count](const auto& x) -> decltype(auto) {
+            ++projection_count;
+            return x.get();
+        });
+    (void) clamped;
+#ifdef _DEBUG
+    ASSERT(projection_count == 5);
+#else
+    ASSERT(projection_count == 3);
+#endif
+    ASSERT(val == "meow");
+}
+
 int main() {
     STATIC_ASSERT((nonrange_tests(), true));
     nonrange_tests();
@@ -350,4 +376,6 @@ int main() {
 
     STATIC_ASSERT((mm_constexpr_tests(), true));
     test_in<mm, const P>();
+
+    test_gh_1893();
 }