[libc-commits] [libc] [libc++] Prevent calling the projection more than three times (PR #66315)

Jocelyn Castellano via libc-commits libc-commits at lists.llvm.org
Mon Oct 2 19:39:34 PDT 2023


https://github.com/pandaninjas updated https://github.com/llvm/llvm-project/pull/66315

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/22] [libc++] Prevent calling the projection more than three
 times

---
 libcxx/include/__algorithm/ranges_clamp.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 9613f7f37720a6c..ca46675eb4b3041 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,9 +37,10 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    if (std::invoke(__comp, std::invoke(__proj, __value), std::invoke(__proj, __low)))
+    auto &projection = std::invoke(__proj, __value);
+    if (std::invoke(__comp, projection, std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __value)))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), projection))
       return __high;
     else
       return __value;

>From c18d60870ac342a95a5528396a8e0c7b91717cbb Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/22] [libc++] Run clang-format on file

---
 libcxx/include/__algorithm/ranges_clamp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index ca46675eb4b3041..3469a6419b2270f 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,7 +37,7 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto &projection = std::invoke(__proj, __value);
+    auto& projection = std::invoke(__proj, __value);
     if (std::invoke(__comp, projection, std::invoke(__proj, __low)))
       return __low;
     else if (std::invoke(__comp, std::invoke(__proj, __high), projection))

>From b40e791f0e9fedbb19936851e1e71decf00331fa Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/22] [libcxx] CamelCase projection and make variable name
 more descriptive

---
 libcxx/include/__algorithm/ranges_clamp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 3469a6419b2270f..3adb5fa828e1ee5 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,7 +37,7 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto& projection = std::invoke(__proj, __value);
+    auto& ValueProjection = std::invoke(__proj, __value);
     if (std::invoke(__comp, projection, std::invoke(__proj, __low)))
       return __low;
     else if (std::invoke(__comp, std::invoke(__proj, __high), projection))

>From a8907624defa4cc4f47520a2d93a8bd042816aa2 Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/22] [libcxx] properly change variable name

---
 libcxx/include/__algorithm/ranges_clamp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 3adb5fa828e1ee5..3d7a224b3649a3f 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -38,9 +38,9 @@ struct __fn {
                                  "Bad bounds passed to std::ranges::clamp");
 
     auto& ValueProjection = std::invoke(__proj, __value);
-    if (std::invoke(__comp, projection, std::invoke(__proj, __low)))
+    if (std::invoke(__comp, ValueProjection, std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), projection))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), ValueProjection))
       return __high;
     else
       return __value;

>From 15d3b2b79fbd61f97b0312e0913cede36b5b202d Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/22] Apply suggestions from code review

---
 libcxx/include/__algorithm/ranges_clamp.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 3d7a224b3649a3f..a97538e4c0e3f65 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,10 +37,10 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto& ValueProjection = std::invoke(__proj, __value);
-    if (std::invoke(__comp, ValueProjection, std::invoke(__proj, __low)))
+    auto&& __projected = std::invoke(__proj, __value);
+    if (std::invoke(__comp, std::forward(__projected), std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), ValueProjection))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward(__projected)))
       return __high;
     else
       return __value;

>From 67a6c91976c76fe1d399a81af8f45ecb549c62ec Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Thu, 14 Sep 2023 11:09:55 -0700
Subject: [PATCH 06/22] [libc++] add test to ensure projection is not called
 more than 3 times

---
 ..._not_called_more_than_three_times.pass.cpp | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp

diff --git a/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp b/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
new file mode 100644
index 000000000000000..8d0713cd573fb34
--- /dev/null
+++ b/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
@@ -0,0 +1,20 @@
+#include <algorithm>
+#include <assert_macros.h>
+
+int Projection(const int value)
+{
+    static int counter = 0;
+
+    if (counter++ == 3)
+    {
+        TEST_FAIL("projection called more than 3 times")
+    }
+
+    return value;
+}
+
+int main()
+{
+    TEST_DOES_NOT_THROW(std::ranges::clamp(3, 2, 4, std::ranges::less{}, &Projection));
+    return 0;
+}

>From cf56669a8ae02b1cb0bf35ef7a37146615778008 Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Thu, 14 Sep 2023 11:10:41 -0700
Subject: [PATCH 07/22] [libc++] run clang-format on files

---
 ..._not_called_more_than_three_times.pass.cpp | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp b/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
index 8d0713cd573fb34..fd47de4eb390534 100644
--- a/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
+++ b/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
@@ -1,20 +1,17 @@
 #include <algorithm>
 #include <assert_macros.h>
 
-int Projection(const int value)
-{
-    static int counter = 0;
+int Projection(const int value) {
+  static int counter = 0;
 
-    if (counter++ == 3)
-    {
-        TEST_FAIL("projection called more than 3 times")
-    }
+  if (counter++ == 3) {
+    TEST_FAIL("projection called more than 3 times")
+  }
 
-    return value;
+  return value;
 }
 
-int main()
-{
-    TEST_DOES_NOT_THROW(std::ranges::clamp(3, 2, 4, std::ranges::less{}, &Projection));
-    return 0;
+int main() {
+  std::ranges::clamp(3, 2, 4, std::ranges::less{}, &Projection);
+  return 0;
 }

>From 04debcbd954222ce20ad3558f7b9bde42401ce75 Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Thu, 14 Sep 2023 17:48:53 -0700
Subject: [PATCH 08/22] [libc++] use decltype in std::forward

---
 libcxx/include/__algorithm/ranges_clamp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index a97538e4c0e3f65..182aed4fc53cbed 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -38,9 +38,9 @@ struct __fn {
                                  "Bad bounds passed to std::ranges::clamp");
 
     auto&& __projected = std::invoke(__proj, __value);
-    if (std::invoke(__comp, std::forward(__projected), std::invoke(__proj, __low)))
+    if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward(__projected)))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected))
       return __high;
     else
       return __value;

>From a377e11f8c502ca1a6557bf90d48efdec409e2a0 Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 10:56:31 -0700
Subject: [PATCH 09/22] Apply testing changes from code review

---
 .../alg.sorting/alg.clamp/ranges.clamp.pass.cpp |  9 ++++++++-
 ...on_not_called_more_than_three_times.pass.cpp | 17 -----------------
 2 files changed, 8 insertions(+), 18 deletions(-)
 delete mode 100644 libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 036552f75de4eb4..189d8cc4d886e8e 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -108,7 +108,14 @@ constexpr bool test() {
     auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { return x; };
     assert(&std::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == &val);
   }
-
+  { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
+    auto projection_function = [](const int value) -> int {
+      static int counter = 0;
+      assert(counter++ != 3);
+      return value;
+    };
+    std::ranges::clamp(3, 2, 4, std::ranges::less{}, &projection_function);
+  }
   return true;
 }
 
diff --git a/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp b/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
deleted file mode 100644
index fd47de4eb390534..000000000000000
--- a/libcxx/test/std/algorithms/ranges_projection_not_called_more_than_three_times.pass.cpp
+++ /dev/null
@@ -1,17 +0,0 @@
-#include <algorithm>
-#include <assert_macros.h>
-
-int Projection(const int value) {
-  static int counter = 0;
-
-  if (counter++ == 3) {
-    TEST_FAIL("projection called more than 3 times")
-  }
-
-  return value;
-}
-
-int main() {
-  std::ranges::clamp(3, 2, 4, std::ranges::less{}, &Projection);
-  return 0;
-}

>From 4e1857f82ffb001300d7e7e4fdd2421bf9db545a Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 11:21:49 -0700
Subject: [PATCH 10/22] [libcxx] Fix typo in ranges_clamp.h

---
 libcxx/include/__algorithm/ranges_clamp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 182aed4fc53cbed..e6c86207254a19f 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -40,7 +40,7 @@ struct __fn {
     auto&& __projected = std::invoke(__proj, __value);
     if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected)))
       return __high;
     else
       return __value;

>From d2adb7371df960f819d40fdac1a1b28af33c180d Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 11:45:48 -0700
Subject: [PATCH 11/22] [libcxx] fix tests, apply changes from code review

---
 libcxx/include/__algorithm/ranges_clamp.h                   | 6 +++---
 .../algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp  | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index e6c86207254a19f..e3bac65856fe984 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,10 +37,10 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto&& __projected = std::invoke(__proj, __value);
-    if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
+    auto&& __projected = std::forward<decltype(__projected)>(std::invoke(__proj, __value));
+    if (std::invoke(__comp, __projected), std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected)))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), __projected))
       return __high;
     else
       return __value;
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 189d8cc4d886e8e..2ff685f67ac0ce3 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -109,12 +109,12 @@ constexpr bool test() {
     assert(&std::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == &val);
   }
   { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
-    auto projection_function = [](const int value) -> int {
-      static int counter = 0;
+    int counter = 0;
+    auto projection_function = [counter](const int value) -> int {
       assert(counter++ != 3);
       return value;
     };
-    std::ranges::clamp(3, 2, 4, std::ranges::less{}, &projection_function);
+    std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function);
   }
   return true;
 }

>From 403ac820e7a89e12820b021ce643c713e8de17a9 Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 12:54:17 -0700
Subject: [PATCH 12/22] Apply testing changes from code review

---
 .../alg.clamp/ranges.clamp.pass.cpp           | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 2ff685f67ac0ce3..8470af2620af3a1 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -116,6 +116,25 @@ constexpr bool test() {
     };
     std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function);
   }
+  {
+    struct Foo {
+      std::string s;
+    };
+
+    // taking by value is important here
+    auto comparator = [](std::string a, std::string b) {
+      return std::atoi(a.c_str()) < std::atoi(b.c_str());
+    };
+
+    auto projection = [](Foo const& foo) {
+      return foo.s;
+    };
+
+    Foo foo{"12"};
+    Foo high{"10"};
+    Foo low{"1"};
+    assert(std::ranges::clamp(foo, low, high, comparator, projection).s == "10");
+  }
   return true;
 }
 

>From 1f686fc8883af3af6d66216555bc85dc9931ea02 Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 13:32:11 -0700
Subject: [PATCH 13/22] Apply suggestions from code review

Co-authored-by: Louis Dionne <ldionne.2 at gmail.com>
---
 libcxx/include/__algorithm/ranges_clamp.h                     | 4 ++--
 .../algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index e3bac65856fe984..9017354667c80e3 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,8 +37,8 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto&& __projected = std::forward<decltype(__projected)>(std::invoke(__proj, __value));
-    if (std::invoke(__comp, __projected), std::invoke(__proj, __low)))
+    auto&& __projected = std::invoke(__proj, __value);
+    if (std::invoke(__comp, __projected, std::invoke(__proj, __low)))
       return __low;
     else if (std::invoke(__comp, std::invoke(__proj, __high), __projected))
       return __high;
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 8470af2620af3a1..63f8281a2821fee 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -110,7 +110,7 @@ constexpr bool test() {
   }
   { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
     int counter = 0;
-    auto projection_function = [counter](const int value) -> int {
+    auto projection_function = [&counter](const int value) -> int {
       assert(counter++ != 3);
       return value;
     };

>From 7a01f9551684b0d76a2fab8d8a41bd12dcce6bae Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 14:42:24 -0700
Subject: [PATCH 14/22] Apply testing changes from code review

---
 .../std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 63f8281a2821fee..5095b366ef2a843 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -111,10 +111,11 @@ constexpr bool test() {
   { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
     int counter = 0;
     auto projection_function = [&counter](const int value) -> int {
-      assert(counter++ != 3);
+      counter++;
       return value;
     };
     std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function);
+    assert(counter <= 3);
   }
   {
     struct Foo {

>From 3492ce33c88af8f557c20336fbd16b9024cf480d Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Fri, 15 Sep 2023 19:38:54 -0700
Subject: [PATCH 15/22] Fix test

---
 .../std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 5095b366ef2a843..151839b5ec4e163 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -114,7 +114,7 @@ constexpr bool test() {
       counter++;
       return value;
     };
-    std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function);
+    assert(std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function) == 3);
     assert(counter <= 3);
   }
   {

>From 1e4cf37570442e4c2cec0a28524dfe7608c2054c Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Mon, 18 Sep 2023 11:20:02 -0700
Subject: [PATCH 16/22] Revert "Apply suggestions from code review"

This reverts commit 1f686fc8883af3af6d66216555bc85dc9931ea02.
---
 libcxx/include/__algorithm/ranges_clamp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 9017354667c80e3..e3bac65856fe984 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,8 +37,8 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto&& __projected = std::invoke(__proj, __value);
-    if (std::invoke(__comp, __projected, std::invoke(__proj, __low)))
+    auto&& __projected = std::forward<decltype(__projected)>(std::invoke(__proj, __value));
+    if (std::invoke(__comp, __projected), std::invoke(__proj, __low)))
       return __low;
     else if (std::invoke(__comp, std::invoke(__proj, __high), __projected))
       return __high;

>From 12bd63eab24a00da23cf9ee23f7eb2241aee4aef Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Mon, 18 Sep 2023 16:18:53 -0700
Subject: [PATCH 17/22] [libc++] Forward the projected value when calling
 std::invoke

---
 libcxx/include/__algorithm/ranges_clamp.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index e3bac65856fe984..0a350aa3b02572f 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,10 +37,10 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto&& __projected = std::forward<decltype(__projected)>(std::invoke(__proj, __value));
-    if (std::invoke(__comp, __projected), std::invoke(__proj, __low)))
+    auto&& __projected = std::invoke(__proj, __value));
+    if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
       return __low;
-    else if (std::invoke(__comp, std::invoke(__proj, __high), __projected))
+    else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected)))
       return __high;
     else
       return __value;

>From 60643fd72c6a962a339bbbcb66c7d2c3a5fc1622 Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Tue, 19 Sep 2023 09:08:43 -0700
Subject: [PATCH 18/22] [libc++] fix typo

---
 libcxx/include/__algorithm/ranges_clamp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h
index 0a350aa3b02572f..e6c86207254a19f 100644
--- a/libcxx/include/__algorithm/ranges_clamp.h
+++ b/libcxx/include/__algorithm/ranges_clamp.h
@@ -37,7 +37,7 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
                                  "Bad bounds passed to std::ranges::clamp");
 
-    auto&& __projected = std::invoke(__proj, __value));
+    auto&& __projected = std::invoke(__proj, __value);
     if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
       return __low;
     else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected)))

>From 80b24be48b2065f5cf8d932087c10295c70be201 Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Tue, 19 Sep 2023 10:15:46 -0700
Subject: [PATCH 19/22] [libc++] Add tests

---
 .../alg.clamp/ranges.clamp.pass.cpp           | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 151839b5ec4e163..6dec2194ec5273b 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -33,6 +33,29 @@ struct CreateNoComp {
   auto operator()(int) const { return NoComp(); }
 };
 
+struct Comp {
+    bool operator()(const int&& x, const int&& y) const {
+        return x < y;
+    }
+    bool operator()(const int&& x, int& y) const {
+        return x < y;
+    }
+    bool operator()(int& x, const int&& y) const {
+        return x < y;
+    }
+    bool operator()(int& x, int& y) const {
+        return x < y;
+    }
+    bool operator()(std::same_as<const int&> auto && x, std::same_as<const int&> auto && y) const {
+        return x < y;
+    }
+};
+
+struct MoveProj {
+    const int&& operator()(const int& x) const { return std::move(x); }
+};
+
+static_assert(std::indirect_strict_weak_order<Comp, std::projected<const int*, Proj>>);
 static_assert(HasClamp<int, std::ranges::less, std::identity>);
 static_assert(!HasClamp<NoComp>);
 static_assert(!HasClamp<int, NoComp>);
@@ -136,6 +159,9 @@ constexpr bool test() {
     Foo low{"1"};
     assert(std::ranges::clamp(foo, low, high, comparator, projection).s == "10");
   }
+  {
+    assert(std::ranges::clamp(0, 1, 2, Comp{}, Proj{}) == 1);
+  }
   return true;
 }
 

>From c20a15ef1ee065e962f73472e1382277f972ddbf Mon Sep 17 00:00:00 2001
From: PandaNinjas <admin at malwarefight.wip.la>
Date: Wed, 20 Sep 2023 14:43:02 -0700
Subject: [PATCH 20/22] [libc++] fix tests again

---
 .../algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 6dec2194ec5273b..9fe5c82bc4b2e99 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -33,7 +33,7 @@ struct CreateNoComp {
   auto operator()(int) const { return NoComp(); }
 };
 
-struct Comp {
+struct Comparator {
     bool operator()(const int&& x, const int&& y) const {
         return x < y;
     }
@@ -55,7 +55,7 @@ struct MoveProj {
     const int&& operator()(const int& x) const { return std::move(x); }
 };
 
-static_assert(std::indirect_strict_weak_order<Comp, std::projected<const int*, Proj>>);
+static_assert(std::indirect_strict_weak_order<Comparator, std::projected<const int*, MoveProj>>);
 static_assert(HasClamp<int, std::ranges::less, std::identity>);
 static_assert(!HasClamp<NoComp>);
 static_assert(!HasClamp<int, NoComp>);
@@ -160,7 +160,7 @@ constexpr bool test() {
     assert(std::ranges::clamp(foo, low, high, comparator, projection).s == "10");
   }
   {
-    assert(std::ranges::clamp(0, 1, 2, Comp{}, Proj{}) == 1);
+    assert(std::ranges::clamp(0, 1, 2, Comparator{}, MoveProj{}) == 1);
   }
   return true;
 }

>From f97313eadac98e753155b5e05ac0eef16f7fb82d Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Sun, 24 Sep 2023 19:32:21 -0700
Subject: [PATCH 21/22] Remove double move test

---
 .../alg.clamp/ranges.clamp.pass.cpp           | 22 -------------------
 1 file changed, 22 deletions(-)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 9fe5c82bc4b2e99..771169071d77598 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -109,28 +109,6 @@ constexpr bool test() {
     assert(&std::ranges::clamp(val, high, low, Comp{}, &S::prvalue_proj) == &low);
   }
 
-  { // Check that the implementation doesn't cause double moves (which could result from calling the projection on
-    // `value` once and then forwarding the result into the comparator).
-    struct CheckDoubleMove {
-      int i;
-      bool moved = false;
-
-      constexpr explicit CheckDoubleMove(int set_i) : i(set_i) {}
-      constexpr CheckDoubleMove(const CheckDoubleMove&) = default;
-      constexpr CheckDoubleMove(CheckDoubleMove&& rhs) noexcept : i(rhs.i) {
-        assert(!rhs.moved);
-        rhs.moved = true;
-      }
-    };
-
-    auto val = CheckDoubleMove{20};
-    auto low = CheckDoubleMove{10};
-    auto high = CheckDoubleMove{30};
-
-    auto moving_comp = [](CheckDoubleMove lhs, CheckDoubleMove rhs) { return lhs.i < rhs.i; };
-    auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { return x; };
-    assert(&std::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == &val);
-  }
   { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
     int counter = 0;
     auto projection_function = [&counter](const int value) -> int {

>From 77dc1783295dc95b9b4e35b89b2f1c5ff8a6c4c9 Mon Sep 17 00:00:00 2001
From: Jocelyn Castellano <admin at malwarefight.wip.la>
Date: Sun, 24 Sep 2023 21:04:25 -0700
Subject: [PATCH 22/22] [libc++] clang-format

---
 .../alg.clamp/ranges.clamp.pass.cpp           | 34 +++++--------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
index 771169071d77598..cedd5f31816da6f 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
@@ -34,21 +34,11 @@ struct CreateNoComp {
 };
 
 struct Comparator {
-    bool operator()(const int&& x, const int&& y) const {
-        return x < y;
-    }
-    bool operator()(const int&& x, int& y) const {
-        return x < y;
-    }
-    bool operator()(int& x, const int&& y) const {
-        return x < y;
-    }
-    bool operator()(int& x, int& y) const {
-        return x < y;
-    }
-    bool operator()(std::same_as<const int&> auto && x, std::same_as<const int&> auto && y) const {
-        return x < y;
-    }
+    bool operator()(const int&& x, const int&& y) const { return x < y; }
+    bool operator()(const int&& x, int& y) const { return x < y; }
+    bool operator()(int& x, const int&& y) const { return x < y; }
+    bool operator()(int& x, int& y) const { return x < y; }
+    bool operator()(std::same_as<const int&> auto&& x, std::same_as<const int&> auto&& y) const { return x < y; }
 };
 
 struct MoveProj {
@@ -110,7 +100,7 @@ constexpr bool test() {
   }
 
   { // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
-    int counter = 0;
+    int counter              = 0;
     auto projection_function = [&counter](const int value) -> int {
       counter++;
       return value;
@@ -124,22 +114,16 @@ constexpr bool test() {
     };
 
     // taking by value is important here
-    auto comparator = [](std::string a, std::string b) {
-      return std::atoi(a.c_str()) < std::atoi(b.c_str());
-    };
+    auto comparator = [](std::string a, std::string b) { return std::atoi(a.c_str()) < std::atoi(b.c_str()); };
 
-    auto projection = [](Foo const& foo) {
-      return foo.s;
-    };
+    auto projection = [](Foo const& foo) { return foo.s; };
 
     Foo foo{"12"};
     Foo high{"10"};
     Foo low{"1"};
     assert(std::ranges::clamp(foo, low, high, comparator, projection).s == "10");
   }
-  {
-    assert(std::ranges::clamp(0, 1, 2, Comparator{}, MoveProj{}) == 1);
-  }
+  { assert(std::ranges::clamp(0, 1, 2, Comparator{}, MoveProj{}) == 1); }
   return true;
 }
 



More information about the libc-commits mailing list