[libc++] Make sure ranges algorithms and views handle boolean-testable correctly

Before this patch, we would fail to implicitly convert the result of
predicates to bool, which means we'd potentially perform a copy or move
construction of the boolean-testable, which isn't allowed. The same
holds true for comparing iterators against sentinels, which is allowed
to return a boolean-testable type.

We already had tests aiming to ensure correct handling of these types,
but they failed to provide appropriate coverage in several cases due to
guaranteed RVO. This patch fixes the tests, adds tests for missing
algorithms and views, and fixes the actual problems in the code.

Fixes llvm#69074

Author: ldionne

libcxx/include/__algorithm/make_projected.h

@@ -96,7 +96,7 @@ decltype(auto) __make_projected_comp(_Comp& __comp, _Proj1& __proj1, _Proj2& __p
     return __comp;
 
   } else {
-    return [&](auto&& __lhs, auto&& __rhs) {
+    return [&](auto&& __lhs, auto&& __rhs) -> bool {
       return std::invoke(__comp,
                         std::invoke(__proj1, std::forward<decltype(__lhs)>(__lhs)),
                         std::invoke(__proj2, std::forward<decltype(__rhs)>(__rhs)));

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);
   }
 };

libcxx/include/__algorithm/ranges_max.h

@@ -56,7 +56,7 @@ struct __fn {
   operator()(initializer_list<_Tp> __il, _Comp __comp = {}, _Proj __proj = {}) const {
     _LIBCPP_ASSERT_UNCATEGORIZED(__il.begin() != __il.end(), "initializer_list must contain at least one element");
 
-    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
+    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
     return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);
   }
 
@@ -72,7 +72,9 @@ struct __fn {
     _LIBCPP_ASSERT_UNCATEGORIZED(__first != __last, "range must contain at least one element");
 
     if constexpr (forward_range<_Rp> && !__is_cheap_to_copy<range_value_t<_Rp>>) {
-      auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
+      auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool {
+        return std::invoke(__comp, __rhs, __lhs);
+      };
       return *ranges::__min_element_impl(std::move(__first), std::move(__last), __comp_lhs_rhs_swapped, __proj);
     } else {
       range_value_t<_Rp> __result = *__first;

libcxx/include/__algorithm/ranges_max_element.h

@@ -37,7 +37,7 @@ struct __fn {
             indirect_strict_weak_order<projected<_Ip, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Ip
   operator()(_Ip __first, _Sp __last, _Comp __comp = {}, _Proj __proj = {}) const {
-    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
+    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
     return ranges::__min_element_impl(__first, __last, __comp_lhs_rhs_swapped, __proj);
   }
 
@@ -46,7 +46,7 @@ struct __fn {
             indirect_strict_weak_order<projected<iterator_t<_Rp>, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>
   operator()(_Rp&& __r, _Comp __comp = {}, _Proj __proj = {}) const {
-    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
+    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };
     return ranges::__min_element_impl(ranges::begin(__r), ranges::end(__r), __comp_lhs_rhs_swapped, __proj);
   }
 };

libcxx/include/__algorithm/ranges_remove.h

@@ -36,7 +36,7 @@ struct __fn {
     requires indirect_binary_predicate<ranges::equal_to, projected<_Iter, _Proj>, const _Type*>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr subrange<_Iter>
   operator()(_Iter __first, _Sent __last, const _Type& __value, _Proj __proj = {}) const {
-    auto __pred = [&](auto&& __other) { return __value == __other; };
+    auto __pred = [&](auto&& __other) -> bool { return __value == __other; };
     return ranges::__remove_if_impl(std::move(__first), std::move(__last), __pred, __proj);
   }
 
@@ -45,7 +45,7 @@ struct __fn {
              indirect_binary_predicate<ranges::equal_to, projected<iterator_t<_Range>, _Proj>, const _Type*>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_subrange_t<_Range>
   operator()(_Range&& __range, const _Type& __value, _Proj __proj = {}) const {
-    auto __pred = [&](auto&& __other) { return __value == __other; };
+    auto __pred = [&](auto&& __other) -> bool { return __value == __other; };
     return ranges::__remove_if_impl(ranges::begin(__range), ranges::end(__range), __pred, __proj);
   }
 };

libcxx/include/__algorithm/ranges_remove_copy.h

@@ -47,7 +47,7 @@ struct __fn {
              indirect_binary_predicate<ranges::equal_to, projected<_InIter, _Proj>, const _Type*>
   _LIBCPP_HIDE_FROM_ABI constexpr remove_copy_result<_InIter, _OutIter>
   operator()(_InIter __first, _Sent __last, _OutIter __result, const _Type& __value, _Proj __proj = {}) const {
-    auto __pred = [&](auto&& __val) { return __value == __val; };
+    auto __pred = [&](auto&& __val) -> bool { return __value == __val; };
     return ranges::__remove_copy_if_impl(std::move(__first), std::move(__last), std::move(__result), __pred, __proj);
   }
 
@@ -56,7 +56,7 @@ struct __fn {
              indirect_binary_predicate<ranges::equal_to, projected<iterator_t<_Range>, _Proj>, const _Type*>
   _LIBCPP_HIDE_FROM_ABI constexpr remove_copy_result<borrowed_iterator_t<_Range>, _OutIter>
   operator()(_Range&& __range, _OutIter __result, const _Type& __value, _Proj __proj = {}) const {
-    auto __pred = [&](auto&& __val) { return __value == __val; };
+    auto __pred = [&](auto&& __val) -> bool { return __value == __val; };
     return ranges::__remove_copy_if_impl(
         ranges::begin(__range), ranges::end(__range), std::move(__result), __pred, __proj);
   }

libcxx/include/__algorithm/ranges_replace.h

@@ -36,7 +36,7 @@ struct __fn {
              indirect_binary_predicate<ranges::equal_to, projected<_Iter, _Proj>, const _Type1*>
   _LIBCPP_HIDE_FROM_ABI constexpr _Iter operator()(
       _Iter __first, _Sent __last, const _Type1& __old_value, const _Type2& __new_value, _Proj __proj = {}) const {
-    auto __pred = [&](const auto& __val) { return __val == __old_value; };
+    auto __pred = [&](const auto& __val) -> bool { return __val == __old_value; };
     return ranges::__replace_if_impl(std::move(__first), std::move(__last), __pred, __new_value, __proj);
   }
 
@@ -45,7 +45,7 @@ struct __fn {
              indirect_binary_predicate<ranges::equal_to, projected<iterator_t<_Range>, _Proj>, const _Type1*>
   _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Range>
   operator()(_Range&& __range, const _Type1& __old_value, const _Type2& __new_value, _Proj __proj = {}) const {
-    auto __pred = [&](auto&& __val) { return __val == __old_value; };
+    auto __pred = [&](auto&& __val) -> bool { return __val == __old_value; };
     return ranges::__replace_if_impl(ranges::begin(__range), ranges::end(__range), __pred, __new_value, __proj);
   }
 };

libcxx/include/__algorithm/ranges_replace_copy.h

@@ -53,7 +53,7 @@ struct __fn {
              const _OldType& __old_value,
              const _NewType& __new_value,
              _Proj __proj = {}) const {
-    auto __pred = [&](const auto& __value) { return __value == __old_value; };
+    auto __pred = [&](const auto& __value) -> bool { return __value == __old_value; };
     return ranges::__replace_copy_if_impl(
         std::move(__first), std::move(__last), std::move(__result), __pred, __new_value, __proj);
   }
@@ -68,7 +68,7 @@ struct __fn {
   _LIBCPP_HIDE_FROM_ABI constexpr replace_copy_result<borrowed_iterator_t<_Range>, _OutIter> operator()(
       _Range&& __range, _OutIter __result, const _OldType& __old_value, const _NewType& __new_value, _Proj __proj = {})
       const {
-    auto __pred = [&](const auto& __value) { return __value == __old_value; };
+    auto __pred = [&](const auto& __value) -> bool { return __value == __old_value; };
     return ranges::__replace_copy_if_impl(
         ranges::begin(__range), ranges::end(__range), std::move(__result), __pred, __new_value, __proj);
   }

libcxx/include/__algorithm/ranges_upper_bound.h

@@ -39,7 +39,7 @@ struct __fn {
             indirect_strict_weak_order<const _Type*, projected<_Iter, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Iter
   operator()(_Iter __first, _Sent __last, const _Type& __value, _Comp __comp = {}, _Proj __proj = {}) const {
-    auto __comp_lhs_rhs_swapped = [&](const auto& __lhs, const auto& __rhs) {
+    auto __comp_lhs_rhs_swapped = [&](const auto& __lhs, const auto& __rhs) -> bool {
       return !std::invoke(__comp, __rhs, __lhs);
     };
 
@@ -52,7 +52,7 @@ struct __fn {
             indirect_strict_weak_order<const _Type*, projected<iterator_t<_Range>, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Range>
   operator()(_Range&& __r, const _Type& __value, _Comp __comp = {}, _Proj __proj = {}) const {
-    auto __comp_lhs_rhs_swapped = [&](const auto& __lhs, const auto& __rhs) {
+    auto __comp_lhs_rhs_swapped = [&](const auto& __lhs, const auto& __rhs) -> bool {
       return !std::invoke(__comp, __rhs, __lhs);
     };
 

libcxx/include/__memory/ranges_uninitialized_algorithms.h

@@ -196,7 +196,7 @@ struct __fn {
   operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
     using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
 
-    auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
+    auto __stop_copying = [&__olast](auto&& __out_iter) -> bool { return __out_iter == __olast; };
     auto __result       = std::__uninitialized_copy<_ValueType>(
         std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_copying);
     return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
@@ -233,7 +233,7 @@ struct __fn {
   operator()(_InputIterator __ifirst, iter_difference_t<_InputIterator> __n,
              _OutputIterator __ofirst, _Sentinel __olast) const {
     using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
-    auto __stop_copying = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
+    auto __stop_copying = [&__olast](auto&& __out_iter) -> bool { return __out_iter == __olast; };
     auto __result =
         std::__uninitialized_copy_n<_ValueType>(std::move(__ifirst), __n, std::move(__ofirst), __stop_copying);
     return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
@@ -263,7 +263,7 @@ struct __fn {
   operator()(_InputIterator __ifirst, _Sentinel1 __ilast, _OutputIterator __ofirst, _Sentinel2 __olast) const {
     using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
     auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
-    auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
+    auto __stop_moving = [&__olast](auto&& __out_iter) -> bool { return __out_iter == __olast; };
     auto __result      = std::__uninitialized_move<_ValueType>(
         std::move(__ifirst), std::move(__ilast), std::move(__ofirst), __stop_moving, __iter_move);
     return {_VSTD::move(__result.first), _VSTD::move(__result.second)};
@@ -301,7 +301,7 @@ struct __fn {
              _OutputIterator __ofirst, _Sentinel __olast) const {
     using _ValueType = remove_reference_t<iter_reference_t<_OutputIterator>>;
     auto __iter_move = [](auto&& __iter) -> decltype(auto) { return ranges::iter_move(__iter); };
-    auto __stop_moving = [&__olast](auto&& __out_iter) { return __out_iter == __olast; };
+    auto __stop_moving = [&__olast](auto&& __out_iter) -> bool { return __out_iter == __olast; };
     auto __result      = std::__uninitialized_move_n<_ValueType>(
         std::move(__ifirst), __n, std::move(__ofirst), __stop_moving, __iter_move);
     return {_VSTD::move(__result.first), _VSTD::move(__result.second)};

libcxx/include/__ranges/chunk_by_view.h

@@ -16,8 +16,6 @@
 #include <__config>
 #include <__functional/bind_back.h>
 #include <__functional/invoke.h>
-#include <__functional/not_fn.h>
-#include <__functional/reference_wrapper.h>
 #include <__iterator/concepts.h>
 #include <__iterator/default_sentinel.h>
 #include <__iterator/iterator_traits.h>
@@ -69,10 +67,11 @@ class chunk_by_view : public view_interface<chunk_by_view<_View, _Pred>> {
   _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_View> __find_next(iterator_t<_View> __current) {
     _LIBCPP_ASSERT_UNCATEGORIZED(
         __pred_.__has_value(), "Trying to call __find_next() on a chunk_by_view that does not have a valid predicate.");
-
-    return ranges::next(ranges::adjacent_find(__current, ranges::end(__base_), std::not_fn(std::ref(*__pred_))),
-                        1,
-                        ranges::end(__base_));
+    auto __reversed_pred = [this]<class _Tp, class _Up>(_Tp&& __x, _Up&& __y) -> bool {
+      return !std::invoke(*__pred_, std::forward<_Tp>(__x), std::forward<_Up>(__y));
+    };
+    return ranges::next(
+        ranges::adjacent_find(__current, ranges::end(__base_), __reversed_pred), 1, ranges::end(__base_));
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_View> __find_prev(iterator_t<_View> __current)
@@ -85,7 +84,7 @@ class chunk_by_view : public view_interface<chunk_by_view<_View, _Pred>> {
 
     auto __first = ranges::begin(__base_);
     reverse_view __reversed{subrange{__first, __current}};
-    auto __reversed_pred = [this]<class _Tp, class _Up>(_Tp&& __x, _Up&& __y) {
+    auto __reversed_pred = [this]<class _Tp, class _Up>(_Tp&& __x, _Up&& __y) -> bool {
       return !std::invoke(*__pred_, std::forward<_Up>(__y), std::forward<_Tp>(__x));
     };
     return ranges::prev(ranges::adjacent_find(__reversed, __reversed_pred).base(), 1, std::move(__first));

libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges.remove.pass.cpp

@@ -25,7 +25,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 template <class Iter, class Sent = sentinel_wrapper<Iter>>

libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges.remove_if.pass.cpp

@@ -25,7 +25,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct FalsePredicate {
@@ -197,20 +196,6 @@ constexpr bool test() {
     }
   }
 
-  {
-    // check that the implicit conversion to bool works
-    {
-      int a[]  = {1, 2, 3, 4};
-      auto ret = std::ranges::remove_if(a, a + 4, [](const int& i) { return BooleanTestable{i == 3}; });
-      assert(ret.begin() == a + 3);
-    }
-    {
-      int a[]  = {1, 2, 3, 4};
-      auto ret = std::ranges::remove_if(a, [](const int& b) { return BooleanTestable{b == 3}; });
-      assert(ret.begin() == a + 3);
-    }
-  }
-
   return true;
 }
 // clang-format on

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/ranges.replace.pass.cpp

@@ -27,7 +27,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 template <class Iter, class Sent = sentinel_wrapper<Iter>>

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/ranges.replace_if.pass.cpp

@@ -26,7 +26,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct FalsePredicate {
@@ -133,19 +132,6 @@ constexpr bool test() {
     }
   }
 
-  { // check that the implicit conversion to bool works
-    {
-      int a[] = {1, 2, 2, 4};
-      auto ret = std::ranges::replace_if(std::begin(a), std::end(a), [](int) { return BooleanTestable{false}; }, 2);
-      assert(ret == std::end(a));
-    }
-    {
-      int a[] = {1, 2, 2, 4};
-      auto ret = std::ranges::replace_if(a, [](int) { return BooleanTestable{false}; }, 2);
-      assert(ret == std::end(a));
-    }
-  }
-
   { // check that std::ranges::dangling is returned
     [[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) ret =
         std::ranges::replace_if(std::array {1, 2, 3, 4}, [](int) { return false; }, 1);

libcxx/test/std/algorithms/alg.nonmodifying/alg.adjacent.find/ranges.adjacent_find.pass.cpp

@@ -26,7 +26,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 template <class Iter, class Sent = Iter>
@@ -172,19 +171,6 @@ constexpr bool test() {
     }
   }
 
-  { // check that the implicit conversion to bool works
-    {
-      int a[] = {1, 2, 2, 4};
-      auto ret = std::ranges::adjacent_find(a, a + 4, [](int i, int j) { return BooleanTestable{i == j}; });
-      assert(ret == a + 1);
-    }
-    {
-      int a[] = {1, 2, 2, 4};
-      auto ret = std::ranges::adjacent_find(a, [](int i, int j) { return BooleanTestable{i == j}; });
-      assert(ret == a + 1);
-    }
-  }
-
   return true;
 }
 

libcxx/test/std/algorithms/alg.nonmodifying/alg.find.first.of/ranges.find_first_of.pass.cpp

@@ -30,7 +30,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 template <class Iter1, class Iter2 = int*, class Sent1 = Iter1, class Sent2 = Iter2>
@@ -194,19 +193,6 @@ constexpr bool test() {
     }
   }
 
-  { // check that the implicit conversion to bool works
-    StrictComparable<int> a[] = {1, 2, 3, 4};
-    StrictComparable<int> b[] = {2, 3};
-    {
-      auto ret = std::ranges::find_first_of(a, std::end(a), b, std::end(b));
-      assert(ret == a + 1);
-    }
-    {
-      auto ret = std::ranges::find_first_of(a, b);
-      assert(ret == a + 1);
-    }
-  }
-
   { // check that the complexity requirements are met
     int a[] = {1, 2, 3, 4};
     int b[] = {2, 3};

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find.pass.cpp

@@ -27,7 +27,6 @@
 #include <vector>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct NotEqualityComparable {};

libcxx/test/std/algorithms/alg.nonmodifying/alg.find/ranges.find_if.pass.cpp

@@ -24,7 +24,6 @@
 #include <ranges>
 
 #include "almost_satisfies_types.h"
-#include "boolean_testable.h"
 #include "test_iterators.h"
 
 struct Predicate {
@@ -224,20 +223,6 @@ constexpr bool test() {
     }
   }
 
-  {
-    // check that the implicit conversion to bool works
-    {
-      int a[] = {1, 2, 3, 4};
-      auto ret = std::ranges::find_if(a, a + 4, [](const int& i) { return BooleanTestable{i == 3}; });
-      assert(ret == a + 2);
-    }
-    {
-      int a[] = {1, 2, 3, 4};
-      auto ret = std::ranges::find_if(a, [](const int& b) { return BooleanTestable{b == 3}; });
-      assert(ret == a + 2);
-    }
-  }
-
   return true;
 }