[llvm-branch-commits] [libcxx] 0e54039 - [libc++] Fix ranges::binary_search() returning true for cases where the element is not in the range

[Tom Stellard via llvm-branch-commits]

Author: Nikolas Klauser
Date: 2023-03-08T05:37:58-08:00
New Revision: 0e540397b083ae8d88c9057f801af30a974e2a4c

URL: https://github.com/llvm/llvm-project/commit/0e540397b083ae8d88c9057f801af30a974e2a4c
DIFF: https://github.com/llvm/llvm-project/commit/0e540397b083ae8d88c9057f801af30a974e2a4c.diff

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

Fixes #61160

Reviewed By: ldionne, #libc

Spies: libcxx-commits

Differential Revision: https://reviews.llvm.org/D145287

(cherry picked from commit 33912468a7dbd2cdef3648878ccd6b8f99e0b5bf)

Added: 
    

Modified: 
    libcxx/include/__algorithm/ranges_binary_search.h
    libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp
    libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp
    libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__algorithm/ranges_binary_search.h b/libcxx/include/__algorithm/ranges_binary_search.h
index b2a8977652fb9..d72d4e057401c 100644
--- a/libcxx/include/__algorithm/ranges_binary_search.h
+++ b/libcxx/include/__algorithm/ranges_binary_search.h
@@ -36,7 +36,7 @@ struct __fn {
   _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,
@@ -46,7 +46,7 @@ struct __fn {
     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

diff  --git a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp
index a1abdaff28b4c..cbeb7eb6fab53 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp
@@ -131,6 +131,13 @@ constexpr void test_iterators() {
     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() {

diff  --git a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp
index e7af57bd0156f..cffce51334440 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/ranges.lower_bound.pass.cpp
@@ -194,6 +194,22 @@ constexpr void test_iterators() {
       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() {

diff  --git a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp
index 762fd7808389c..cab7831c2b59b 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/ranges.upper_bound.pass.cpp
@@ -193,6 +193,22 @@ constexpr void test_iterators() {
       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() {