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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
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  = 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 
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  = 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 
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 
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 
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 

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

[Louis Dionne via cfe-commits]

ldionne wrote:

This seems to be a bit stuck. I checked out the patch locally and that was 
helpful to understand some of the issues. I think the final patch should be:

```
commit c667f036c4228b9cf0172c311537ab96ef61fcf5
Author: PandaNinjas 
Date:   Wed Sep 13 17:38:17 2023 -0700

[libc++] Prevent calling the projection more than three times

Resolves #64717

diff --git a/libcxx/include/__algorithm/ranges_clamp.h 
b/libcxx/include/__algorithm/ranges_clamp.h
index 9613f7f37720..e6c86207254a 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&& __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), 
std::invoke(__proj, __value)))
+else if (std::invoke(__comp, std::invoke(__proj, __high), 
std::forward(__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 036552f75de4..35ef55a91e24 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
@@ -38,6 +38,16 @@ static_assert(!HasClamp);
 static_assert(!HasClamp);
 static_assert(!HasClamp);
 
+struct EnsureValueCategoryComp {
+  constexpr bool operator()(const int&& x, const int&& y) const { return x < 
y; }
+  constexpr bool operator()(const int&& x, int& y) const { return x < y; }
+  constexpr bool operator()(int& x, const int&& y) const { return x < y; }
+  constexpr bool operator()(int& x, int& y) const { return x < y; }
+  constexpr bool operator()(std::same_as auto&& x, 
std::same_as auto&& y) const {
+return x < y;
+  }
+};
+
 constexpr bool test() {
   { // low < val < high
 int val = 2;
@@ -71,6 +81,7 @@ constexpr bool test() {
 
   constexpr const int& lvalue_proj() const { return i; }
   constexpr int prvalue_proj() const { return i; }
+  constexpr bool operator==(S const& other) const { return i == other.i; }
 };
 
 struct Comp {
@@ -82,31 +93,29 @@ constexpr bool test() {
 auto low = S{20};
 auto high = S{30};
 // Check that the value category of the projection return type is 
preserved.
-assert(::ranges::clamp(val, low, high, Comp{}, ::lvalue_proj) == 
);
-assert(::ranges::clamp(val, high, low, Comp{}, ::prvalue_proj) == 
);
+assert(std::ranges::clamp(val, low, high, Comp{}, ::lvalue_proj) == low);
+assert(std::ranges::clamp(val, high, low, Comp{}, ::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;
-  }
+  { // Ensure that we respect the value category of the projection when 
calling the comparator.
+// This additional example was provided by Tim Song in 
https://github.com/microsoft/STL/issues/3970#issuecomment-1685120958.
+struct MoveProj {
+  constexpr int const&& operator()(int const& x) const { return 
std::move(x); }
 };
 
-auto val = CheckDoubleMove{20};
-auto low = CheckDoubleMove{10};
-auto high = CheckDoubleMove{30};
+static_assert(std::indirect_strict_weak_order>);
 
-auto moving_comp = [](CheckDoubleMove lhs, CheckDoubleMove rhs) { return 
lhs.i < rhs.i; };
-auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
-assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
+assert(std::ranges::clamp(0, 1, 2, EnsureValueCategoryComp{}, MoveProj{}) 
== 1);
+  }
+
+  { // Make sure we don't call the projection more than three times per 
[alg.clamp], see #64717
+int counter  = 0;
+auto projection_function = [](const int value) -> int {
+  counter++;
+  return value;
+};
+assert(std::ranges::clamp(3, 2, 4, std::ranges::less{}, 
projection_function) == 3);
+assert(counter <= 3);
   }
 
   return true;

```

Can you update the PR accordingly? Also, in the future I believe that following 
the instructions in 

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff c60ccfbb898148449946f82cbac6140f1e01cb12 
f97313eadac98e753155b5e05ac0eef16f7fb82d -- 
libcxx/include/__algorithm/ranges_clamp.h 
libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp
``





View the diff from clang-format here.


``diff
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 771169071d77..69d2af0fdf62 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,25 +34,15 @@ 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 auto && x, std::same_as 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 auto&& x, std::same_as 
auto&& y) const { return x < y; }
 };
 
 struct MoveProj {
-const int&& operator()(const int& x) const { return std::move(x); }
+  const int&& operator()(const int& x) const { return std::move(x); }
 };
 
 static_assert(std::indirect_strict_weak_order>);
@@ -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 = [](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;
 }
 

``




https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[A. Jiang via cfe-commits]

frederick-vs-ja wrote:

It seems that we should remove this block because it turns out that 
`ranges::clamp` needs double moves under some circumstances.
https://github.com/llvm/llvm-project/blob/4c1c96e6fc0f704e9e032f87b2cd1e04bb4240dd/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp#L89-L110

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

pandaninjas wrote:

It seems like tests are still failing because there is a double move.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/18] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/18] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/18] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/18] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/18] 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 

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/17] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/17] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/17] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/17] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/17] 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 

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -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&& __projected = 
std::forward(std::invoke(__proj, __value));

ldionne wrote:

We don't want to use `std::forward` here, we want to use it in the argument to 
`std::invoke`.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -108,7 +108,34 @@ constexpr bool test() {
 auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
 assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
   }
+  { // Make sure we don't call the projection more than three times per 
[alg.clamp], see #64717
+int counter = 0;
+auto projection_function = [](const int value) -> int {
+  counter++;
+  return value;
+};
+assert(std::ranges::clamp(3, 2, 4, std::ranges::less{}, 
projection_function) == 3);
+assert(counter <= 3);
+  }
+  {
+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");
+  }

ldionne wrote:

^ This test is still missing IIUC.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/16] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/16] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/16] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/16] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/16] 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 

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

[Louis Dionne via cfe-commits]

@@ -108,7 +108,34 @@ constexpr bool test() {
 auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
 assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
   }
+  { // Make sure we don't call the projection more than three times per 
[alg.clamp], see #64717
+int counter = 0;
+auto projection_function = [](const int value) -> int {
+  counter++;
+  return value;
+};
+assert(std::ranges::clamp(3, 2, 4, std::ranges::less{}, 
projection_function) == 3);
+assert(counter <= 3);
+  }
+  {
+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");
+  }

ldionne wrote:

Let's add the test case from https://gcc.godbolt.org/z/a8ne6e487.

In this case it means that we also need to use `std::forward` like you did 
previously, thanks @timsong-cpp for clarifying.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

ldionne wrote:

> Any idea why the CI is failing? It mentions line 86 not being constexpr 
> because of assert, but it doesn't seem like I've done anything which should 
> affect that...

I looked into it a bit and I think the CI is failing because of the 
`std::forward` problem. The assertion on line 86 fails because we don't respect 
the value category returned by the projection, which is basically that problem.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

pandaninjas wrote:

Any idea why the CI is failing? It mentions line 86 not being constexpr because 
of assert, but it doesn't seem like I've done anything which should affect 
that...

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/15] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/15] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/15] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/15] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/15] 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 

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/14] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/14] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/14] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/14] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/14] 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 

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

[Louis Dionne via cfe-commits]

@@ -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&& __projected = std::invoke(__proj, __value);

ldionne wrote:

I don't see how it could dangle. Do you mean that it will dangle if 
`std::invoke(__proj, __value)` returns a temporary? Rvalue references extend 
the lifetime of temporaries so whatever this references will live until the end 
of the function.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -108,7 +108,33 @@ constexpr bool test() {
 auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
 assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
   }
+  { // 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);

ldionne wrote:

I think you missed this comment!

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

https://github.com/EricWF edited https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -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&& __projected = std::invoke(__proj, __value);

EricWF wrote:

This could dangle. 

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

https://github.com/EricWF requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/13] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/13] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/13] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/13] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/13] 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 

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

[Jocelyn Castellano via cfe-commits]

https://github.com/pandaninjas resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

https://github.com/pandaninjas resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

https://github.com/pandaninjas resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

ldionne wrote:

What a tricky change! The ones that seem simple are sometimes the ones that end 
up being the most involved :)

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -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&& __projected = 
std::forward(std::invoke(__proj, __value));
+if (std::invoke(__comp, __projected), std::invoke(__proj, __low)))

ldionne wrote:

```suggestion
if (std::invoke(__comp, __projected, std::invoke(__proj, __low)))
```

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -108,7 +108,33 @@ constexpr bool test() {
 auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
 assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
   }
+  { // 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 {

ldionne wrote:

```suggestion
auto projection_function = [](const int value) -> int {
```

Otherwise the test might not work if we copy the projection itself!

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -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&& __projected = 
std::forward(std::invoke(__proj, __value));

ldionne wrote:

```suggestion
auto&& __projected = std::invoke(__proj, __value);
```

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -108,7 +108,33 @@ constexpr bool test() {
 auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { 
return x; };
 assert(::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == 
);
   }
+  { // 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);

ldionne wrote:

I would just do `++counter` here, no assertion. And then I would check after 
calling `std::ranges::clamp` that `assert(counter == 3)`.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -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&& __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), 
std::invoke(__proj, __value)))
+else if (std::invoke(__comp, std::invoke(__proj, __high), 
std::forward(__projected))

ldionne wrote:

Ok, so actually it turns out that MSVC does the same thing here and they went 
through the exact same questioning as us, see 
https://github.com/microsoft/STL/pull/1898#discussion_r625118642.

However, I [played around](https://godbolt.org/z/fMM7nrn74) and I wasn't able 
to craft a case where passing an lvalue would fail (despite what @timsong-cpp 
said in that review). Unless we are able to come up with one, I think it would 
be reasonable to pass a lvalue.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/12] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/12] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/12] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/12] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/12] 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 

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/11] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/11] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/11] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/11] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/11] 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 

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 01/10] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 02/10] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 03/10] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 04/10] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 05/10] 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 

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 1/9] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 2/9] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 3/9] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 4/9] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 5/9] 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
--- 

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -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&& __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), 
std::invoke(__proj, __value)))
+else if (std::invoke(__comp, std::invoke(__proj, __high), 
std::forward(__projected))

ldionne wrote:

There is actually a problem here. We can potentially do a double-move. Imagine 
we have the following comparator:

```
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"};
std::ranges::clamp(foo, low, high, comparator, projection);
```

Here, we end up calling `std::invoke(comp, forward<...>(projected), low)`, it 
returns false and then we call `std::invoke(comp, forward<...>(projected), 
high)`. We end up moving `projected` twice into the `invoke` calls, so the 
second time we get an empty string.

Is that right? If so, this would need a test too.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

https://github.com/ldionne resolved 
https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via cfe-commits]

@@ -37,9 +37,10 @@ struct __fn {
 _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, 
__high), std::invoke(__proj, __low))),

ldionne wrote:

You're right, sorry for the noise. It supports a custom comparator, not a 
custom projection. I read too fast.

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 1/9] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 2/9] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 3/9] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 4/9] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 5/9] 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
--- 

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

[Jocelyn Castellano via cfe-commits]

@@ -37,9 +37,10 @@ struct __fn {
 _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, 
__high), std::invoke(__proj, __low))),

pandaninjas wrote:

I think we don't need a similar fix in `std::clamp` because `std::clamp` 
doesn't support using a projection

https://github.com/llvm/llvm-project/pull/66315
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jocelyn Castellano via cfe-commits]

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

>From ead65bfcb70be46788bc9e88c891e7ae7f91b8d7 Mon Sep 17 00:00:00 2001
From: PandaNinjas 
Date: Wed, 13 Sep 2023 17:38:17 -0700
Subject: [PATCH 1/8] [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  = 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 
Date: Wed, 13 Sep 2023 18:56:44 -0700
Subject: [PATCH 2/8] [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  = 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 
Date: Wed, 13 Sep 2023 19:11:20 -0700
Subject: [PATCH 3/8] [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 
Date: Wed, 13 Sep 2023 19:11:47 -0700
Subject: [PATCH 4/8] [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 
Date: Thu, 14 Sep 2023 10:37:34 -0700
Subject: [PATCH 5/8] 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
---