[libcxx-commits] [libcxx] [libc++] Make sure ranges::find_if_not handles boolean-testables correctly (PR #69378)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

<details>
<summary>Changes</summary>

We would fail to implicitly convert the result of the predicate to bool, which means we'd potentially perform a copy or move construction of the boolean-testable, which isn't allowed. We already had tests aiming to ensure correct handling of these types, but they failed to catch copy and move construction because of guaranteed RVO.

Fixes #<!-- -->69074

---
Full diff: https://github.com/llvm/llvm-project/pull/69378.diff


3 Files Affected:

- (modified) libcxx/include/__algorithm/ranges_find_if_not.h (+2-2) 
- (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if_not.pass.cpp (+1-1) 
- (modified) libcxx/test/support/boolean_testable.h (+6-3) 


``````````diff
diff --git a/libcxx/include/__algorithm/ranges_find_if_not.h b/libcxx/include/__algorithm/ranges_find_if_not.h
index 6beade1462e099c..a18bea43165e0d8 100644
--- a/libcxx/include/__algorithm/ranges_find_if_not.h
+++ b/libcxx/include/__algorithm/ranges_find_if_not.h
@@ -39,14 +39,14 @@ struct __fn {
             indirect_unary_predicate<projected<_Ip, _Proj>> _Pred>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Ip
   operator()(_Ip __first, _Sp __last, _Pred __pred, _Proj __proj = {}) const {
-    auto __pred2 = [&](auto&& __e) { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };
+    auto __pred2 = [&](auto&& __e) -> bool { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };
     return ranges::__find_if_impl(std::move(__first), std::move(__last), __pred2, __proj);
   }
 
   template <input_range _Rp, class _Proj = identity, indirect_unary_predicate<projected<iterator_t<_Rp>, _Proj>> _Pred>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>
   operator()(_Rp&& __r, _Pred __pred, _Proj __proj = {}) const {
-    auto __pred2 = [&](auto&& __e) { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };
+    auto __pred2 = [&](auto&& __e) -> bool { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };
     return ranges::__find_if_impl(ranges::begin(__r), ranges::end(__r), __pred2, __proj);
   }
 };
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if_not.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if_not.pass.cpp
index 95860745f56204e..03d43ebb752bff2 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if_not.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if_not.pass.cpp
@@ -227,7 +227,7 @@ constexpr bool test() {
     }
     {
       int a[] = {1, 2, 3, 4};
-      auto ret = std::ranges::find_if_not(a, [](const int& b) { return BooleanTestable{b != 3}; });
+      auto ret = std::ranges::find_if_not(a, [](const int& i) { return BooleanTestable{i != 3}; });
       assert(ret == a + 2);
     }
   }
diff --git a/libcxx/test/support/boolean_testable.h b/libcxx/test/support/boolean_testable.h
index e810e4e0461dc69..22bcefe04f9a53c 100644
--- a/libcxx/test/support/boolean_testable.h
+++ b/libcxx/test/support/boolean_testable.h
@@ -11,6 +11,8 @@
 
 #include "test_macros.h"
 
+#include <utility>
+
 #if TEST_STD_VER > 17
 
 class BooleanTestable {
@@ -24,11 +26,12 @@ class BooleanTestable {
   }
 
   friend constexpr BooleanTestable operator!=(const BooleanTestable& lhs, const BooleanTestable& rhs) {
-    return !(lhs == rhs);
+    return lhs.value_ != rhs.value_;
   }
 
-  constexpr BooleanTestable operator!() {
-    return BooleanTestable{!value_};
+  constexpr BooleanTestable&& operator!() && {
+    value_ = !value_;
+    return std::move(*this);
   }
 
   // this class should behave like a bool, so the constructor shouldn't be explicit

``````````

</details>


https://github.com/llvm/llvm-project/pull/69378