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

[Louis Dionne via libcxx-commits]

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

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

>From 733616cfe9ea1c6d53965209475f4ca3e1d6fe22 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Tue, 17 Oct 2023 13:29:22 -0700
Subject: [PATCH] [libc++] Make sure ranges::find_if_not handles
 boolean-testables correctly

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
---
 libcxx/include/__algorithm/ranges_find_if_not.h          | 4 ++--
 .../alg.find/ranges.find_if_not.pass.cpp                 | 2 +-
 libcxx/test/support/boolean_testable.h                   | 9 ++++++---
 3 files changed, 9 insertions(+), 6 deletions(-)

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