[libc++] Fix ranges::binary_search() returning true for cases where the element is not in the range

[philnik]

Summary

Fixes #61160

[Mordante]

libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp

202

Please test the value of *ret too. I know it's kind of double but it makes the test easier to understand.
(The same for upper_bound.)

Do you know why these test don't use std::begin(a) and std::end(a). To me that makes clear it tests the entire range and I don't need to count the elements.

[ldionne]

Let's cherry-pick this one to release/16.x

[philnik]

libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp

202

I think Arthur wanted it that way, but I don't remember exactly why we did it that way.

libcxx/include/__algorithm/ranges_binary_search.h

 namespace ranges {
 namespace __binary_search {
 struct __fn {
   template <forward_iterator _Iter, sentinel_for<_Iter> _Sent, class _Type, class _Proj = identity,
             indirect_strict_weak_order<const _Type*, projected<_Iter, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr
   bool operator()(_Iter __first, _Sent __last, const _Type& __value, _Comp __comp = {}, _Proj __proj = {}) const {
     auto __ret = std::__lower_bound_impl<_RangeAlgPolicy>(__first, __last, __value, __comp, __proj);
-    return __ret != __last && !std::invoke(__comp, __value, std::invoke(__proj, *__first));
+    return __ret != __last && !std::invoke(__comp, __value, std::invoke(__proj, *__ret));
   }
 
   template <forward_range _Range, class _Type, class _Proj = identity,
             indirect_strict_weak_order<const _Type*, projected<iterator_t<_Range>, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr
   bool operator()(_Range&& __r, const _Type& __value, _Comp __comp = {}, _Proj __proj = {}) const {
     auto __first = ranges::begin(__r);
     auto __last = ranges::end(__r);
     auto __ret = std::__lower_bound_impl<_RangeAlgPolicy>(__first, __last, __value, __comp, __proj);
-    return __ret != __last && !std::invoke(__comp, __value, std::invoke(__proj, *__first));
+    return __ret != __last && !std::invoke(__comp, __value, std::invoke(__proj, *__ret));
   }
 };
 } // namespace __binary_search
 
 inline namespace __cpo {
   inline constexpr auto binary_search = __binary_search::__fn{};
 } // namespace __cpo
 } // namespace ranges
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_STD_VER >= 20
 
 #endif // _LIBCPP___ALGORITHM_RANGES_BINARY_SEARCH_H

libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp

constexpr void test_iterators() {
   }
 
   { // check that it works when all but the searched for elements are equal
     int a[] = {1, 2, 2, 2, 2};
     assert(std::ranges::binary_search(It(a), Sent(It(a + 5)), 1));
     auto range = std::ranges::subrange(It(a), Sent(It(a + 5)));
     assert(std::ranges::binary_search(range, 1));
   }
+
+  { // check that false is returned when the element doesn't exist, but an element with a greater value is in the range
+    int a[] = {1, 2, 4};
+    assert(!std::ranges::binary_search(It(a), Sent(It(a + 3)), 3));
+    auto range = std::ranges::subrange(It(a), Sent(It(a + 3)));
+    assert(!std::ranges::binary_search(range, 3));
+  }
 }
 
 constexpr bool test() {
   test_iterators<int*>();
   test_iterators<forward_iterator<int*>>();
   test_iterators<forward_iterator<int*>, sentinel_wrapper<forward_iterator<int*>>>();
   test_iterators<bidirectional_iterator<int*>>();
   test_iterators<random_access_iterator<int*>>();

libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp

constexpr void test_iterators() {
     }
     {
       int a[] = {1, 6, 6, 6, 6, 6};
       auto range = std::ranges::subrange(It(a), It(a + 6));
       auto ret = std::ranges::lower_bound(range, 1);
       assert(base(ret) == a);
     }
   }
+
+  { // check that the middle of a range is returned when there are smaller and larger elements
+    {
+      int a[] = {1, 2, 3, 4, 6, 7, 8};
+      auto ret = std::ranges::lower_bound(It(a), It(a + 7), 5);
+      assert(base(ret) == a + 4);
+      assert(*ret == 6);
+    }
+    {
+      int a[] = {1, 2, 3, 4, 6, 7, 8};
+      auto range = std::ranges::subrange(It(a), It(a + 7));
+      auto ret = std::ranges::lower_bound(range, 5);
+      assert(base(ret) == a + 4);
+      assert(*ret == 6);
+    }
+  }
 }
 
 constexpr bool test() {
   test_iterators<int*>();
   test_iterators<forward_iterator<int*>>();
   test_iterators<bidirectional_iterator<int*>>();
   test_iterators<random_access_iterator<int*>>();
   test_iterators<contiguous_iterator<int*>>();

libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp

constexpr void test_iterators() {
     }
     {
       int a[] = {1, 6, 6, 6, 6, 6};
       auto range = std::ranges::subrange(It(a), It(a + 6));
       auto ret = std::ranges::upper_bound(range, 1);
       assert(base(ret) == a + 1);
     }
   }
+
+  { // check that the middle of a range is returned when there are smaller and larger elements
+    {
+      int a[] = {1, 2, 3, 4, 6, 7, 8};
+      auto ret = std::ranges::upper_bound(It(a), It(a + 7), 5);
+      assert(base(ret) == a + 4);
+      assert(*ret == 6);
+    }
+    {
+      int a[] = {1, 2, 3, 4, 6, 7, 8};
+      auto range = std::ranges::subrange(It(a), It(a + 7));
+      auto ret = std::ranges::upper_bound(range, 5);
+      assert(base(ret) == a + 4);
+      assert(*ret == 6);
+    }
+  }
 }
 
 constexpr bool test() {
   test_iterators<int*>();
   test_iterators<forward_iterator<int*>>();
   test_iterators<bidirectional_iterator<int*>>();
   test_iterators<random_access_iterator<int*>>();
   test_iterators<contiguous_iterator<int*>>();