[libcxx-commits] [PATCH] D121964: [libc++][ranges] Implement ranges::binary_search and ranges::{lower, upper}_bound

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 14 07:11:50 PDT 2022

philnik added inline comments.

Comment at: libcxx/include/__algorithm/ranges_lower_bound.h:38
+_Ip __lower_bound_impl(_Ip __first, _Sp __last, const _Tp& __value, _Comp& __comp, _Proj& __proj) {
+  using __diff_t = typename iterator_traits<_Ip>::difference_type;
var-const wrote:
> Have you considered sharing the implementation of the algorithm between the ranges and non-ranges versions? This algorithm, while not very complicated, isn't trivial and is quite prone to off-by-one errors. Personally, I'm on the fence.
> (cc'ing @ldionne as well)
Yes, but I think it's simple enough to not warrant sharing the implementation, but I'm on the fence too.

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.binary.search/binary.search/ranges.binary_search.pass.cpp:71
+constexpr bool test() {
+  test_iterators<int*>();
var-const wrote:
> Similar to another patch, it probably makes sense to test that the comparator works when:
> - it takes the arguments by non-const lvalue;
> - its `operator()` is not `const`;
> - it returns something convertible to `bool`.
I'll add the convertible to `bool` tests once D122011 landed. Taking only non-`const` lvalues doesn't model `indirect_strict_weak_order`.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list