* Fix C++03 compatibility issues.

* Fix tests I had broken.
* More tweaks and better comments.

Author: ichaer

libcxx/include/__algorithm/iterator_operations.h

@@ -98,7 +98,7 @@ struct _IterOps<_ClassicAlgPolicy> {
   template <class _InputIter, class _Distance>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static _Distance
   __advance(_InputIter& __iter, _Distance __count, const _InputIter& __sentinel, input_iterator_tag) {
-    _Distance __dist{};
+    _Distance __dist = _Distance();
     for (; __dist < __count && __iter != __sentinel; ++__dist)
       ++__iter;
     return __count - __dist;
@@ -108,7 +108,7 @@ struct _IterOps<_ClassicAlgPolicy> {
   template <class _BiDirIter, class _Distance>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 static _Distance
   __advance(_BiDirIter& __iter, _Distance __count, const _BiDirIter& __sentinel, bidirectional_iterator_tag) {
-    _Distance __dist{};
+    _Distance __dist = _Distance();
     if (__count >= 0)
       for (; __dist < __count && __iter != __sentinel; ++__dist)
         ++__iter;
@@ -120,7 +120,7 @@ struct _IterOps<_ClassicAlgPolicy> {
 
   // advance with sentinel, a la std::ranges::advance -- RandomIterator specialization
   template <class _RandIter, class _Distance>
-  _LIBCPP_HIDE_FROM_ABI constexpr static _Distance
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR static _Distance
   __advance(_RandIter& __iter, _Distance __count, const _RandIter& __sentinel, random_access_iterator_tag) {
     auto __dist = _IterOps::distance(__iter, __sentinel);
     _LIBCPP_ASSERT_UNCATEGORIZED(

libcxx/include/__algorithm/lower_bound.h

@@ -50,7 +50,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Iter
 
 // One-sided binary search, aka meta binary search, has been in the public domain for decades, and has the general
 // advantage of being Ω(1) rather than the classic algorithm's Ω(log(n)), with the downside of executing at most
-// 2*(log(n)-1) comparisons vs the classic algorithm's exact log(n). There are two scenarios in which it really shines:
+// 2*log(n) comparisons vs the classic algorithm's exact log(n). There are two scenarios in which it really shines:
 // the first one is when operating over non-random iterators, because the classic algorithm requires knowing the
 // container's size upfront, which adds Ω(n) iterator increments to the complexity. The second one is when you're
 // traversing the container in order, trying to fast-forward to the next value: in that case, the classic algorithm
@@ -63,11 +63,9 @@ __lower_bound_onesided(_Iter __first, _Sent __last, const _Type& __value, _Comp&
   // __iterator_category<_Iter>>::value,
   //       "lower_bound() is a multipass algorithm and requires forward iterator or better");
 
-  // split the step 0 scenario: this allows us to match worst-case complexity
-  // when replacing linear search
+  // step = 0, ensuring we can always short-circuit when distance is 1 later on
   if (__first == __last || !std::__invoke(__comp, std::__invoke(__proj, *__first), __value))
     return __first;
-  ++__first;
 
   using _Distance = typename iterator_traits<_Iter>::difference_type;
   for (_Distance __step = 1; __first != __last; __step <<= 1) {
@@ -76,10 +74,14 @@ __lower_bound_onesided(_Iter __first, _Sent __last, const _Type& __value, _Comp&
     // once we reach the last range where needle can be we must start
     // looking inwards, bisecting that range
     if (__it == __last || !std::__invoke(__comp, std::__invoke(__proj, *__it), __value)) {
+      // we've already checked the previous value and it was less, we can save
+      // one comparison by skipping bisection
+      if (__dist == 1)
+        return __it;
       return std::__lower_bound_bisecting<_AlgPolicy>(__first, __value, __dist, __comp, __proj);
     }
     // range not found, move forward!
-    __first = std::move(__it);
+    __first = __it;
   }
   return __first;
 }

libcxx/include/__algorithm/set_intersection.h

@@ -20,6 +20,7 @@
 #include <__type_traits/is_same.h>
 #include <__utility/exchange.h>
 #include <__utility/move.h>
+#include <__utility/swap.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -50,8 +51,7 @@ struct _LIBCPP_NODISCARD_EXT __set_intersector {
   const _Sent2& __last2_;
   _OutIter& __result_;
   _Compare& __comp_;
-  static constexpr auto __proj_ = std::__identity();
-  bool __prev_advanced_         = true;
+  bool __prev_advanced_ = true;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __set_intersector(
       _InIter1& __first1, _Sent1& __last1, _InIter2& __first2, _Sent2& __last2, _OutIter& __result, _Compare& __comp)
@@ -64,7 +64,7 @@ struct _LIBCPP_NODISCARD_EXT __set_intersector {
 
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
       __set_intersection_result<_InIter1, _InIter2, _OutIter>
-      operator()() && {
+      operator()() {
     while (__first2_ != __last2_) {
       __advance1_and_maybe_add_result();
       if (__first1_ == __last1_)
@@ -85,9 +85,27 @@ struct _LIBCPP_NODISCARD_EXT __set_intersector {
   template <class _Iter, class _Sent, class _Value>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
   __advance_and_maybe_add_result(_Iter& __iter, const _Sent& __sentinel, const _Value& __value) {
-    // use one-sided lower bound for improved algorithmic complexity bounds
-    const auto __tmp = std::move(__iter);
-    __iter           = std::__lower_bound_onesided<_AlgPolicy>(__iter, __sentinel, __value, __comp_, __proj_);
+    static _LIBCPP_CONSTEXPR std::__identity __proj;
+    // use one-sided binary search for improved algorithmic complexity bounds
+    // understanding how we can use binary search and still respect complexity
+    // guarantees is _not_ straightforward, so let me explain: the guarantee
+    // is "at most 2*(N+M)-1 comparisons", and one-sided binary search will
+    // necessarily overshoot depending on the position of the needle in the
+    // haystack -- for instance, if we're searching for 3 in (1, 2, 3, 4),
+    // we'll check if 3<1, then 3<2, then 3<4, and, finally, 3<3, for a total of
+    // 4 comparisons, when linear search would have yielded 3. However,
+    // because we won't need to perform the intervening reciprocal comparisons
+    // (ie 1<3, 2<3, 4<3), that extra comparison doesn't run afoul of the
+    // guarantee. Additionally, this type of scenario can only happen for match
+    // distances of up to 5 elements, because 2*log2(8) is 6, and we'll still
+    // be worse-off at position 5 of an 8-element set. From then onwards
+    // these scenarios can't happen.
+    // TL;DR: we'll be 1 comparison worse-off compared to the classic linear-
+    // searching algorithm if matching position 3 of a set with 4 elements,
+    // or position 5 if the set has 7 or 8 elements, but we'll never exceed
+    // the complexity guarantees from the standard.
+    _Iter __tmp = std::__lower_bound_onesided<_AlgPolicy>(__iter, __sentinel, __value, __comp_, __proj);
+    std::swap(__tmp, __iter);
     __add_output_unless(__tmp != __iter);
   }
 
@@ -137,7 +155,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
         std::forward_iterator_tag) {
   std::__set_intersector<_AlgPolicy, _Compare, _InForwardIter1, _Sent1, _InForwardIter2, _Sent2, _OutIter>
       __intersector(__first1, __last1, __first2, __last2, __result, __comp);
-  return std::move(__intersector)();
+  return __intersector();
 }
 
 // input iterators are not suitable for multipass algorithms, so we stick to the classic single-pass version
@@ -183,7 +201,7 @@ class __set_intersection_iter_category {
   template <class _It>
   using __cat = typename std::_IterOps<_AlgPolicy>::template __iterator_category<_It>;
   template <class _It>
-  static auto test(__cat<_It>*) -> __cat<_It>;
+  static __cat<_It> test(__cat<_It>*);
   template <class>
   static std::input_iterator_tag test(...);
 
@@ -202,7 +220,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
       std::move(__first2),
       std::move(__last2),
       std::move(__result),
-      std::forward<_Compare>(__comp),
+      __comp,
       typename std::__set_intersection_iter_category<_AlgPolicy, _InIter1>::__type(),
       typename std::__set_intersection_iter_category<_AlgPolicy, _InIter2>::__type());
 }

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

@@ -39,20 +39,27 @@ template <class Iter, class T>
 void
 test(Iter first, Iter last, const T& value)
 {
-  std::size_t strides{};
-  std::size_t displacement{};
+#if TEST_STD_VER > 17
+  std::size_t strides      = 0;
+  std::size_t displacement = 0;
   stride_counting_iterator f(first, &strides, &displacement);
   stride_counting_iterator l(last, &strides, &displacement);
+#else
+  Iter& f = first;
+  Iter& l = last;
+#endif
 
   auto i = std::lower_bound(f, l, value);
   for (auto j = base(f); j != base(i); ++j)
     assert(*j < value);
   for (auto j = base(i); j != base(l); ++j)
     assert(!(*j < value));
 
+#if TEST_STD_VER > 17
   auto len = static_cast<std::size_t>(std::distance(first, last));
   assert(strides <= 2 * len);
   assert(displacement <= 2 * len);
+#endif
 }
 
 template <class Iter>

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

@@ -39,16 +39,27 @@ template <class Iter, class T>
 void
 test(Iter first, Iter last, const T& value)
 {
-  std::size_t strides{};
-  std::size_t displacement{};
+#if TEST_STD_VER > 17
+  std::size_t strides      = 0;
+  std::size_t displacement = 0;
   stride_counting_iterator f(first, &strides, &displacement);
   stride_counting_iterator l(last, &strides, &displacement);
+#else
+  Iter& f = first;
+  Iter& l = last;
+#endif
+
+  std::size_t comparisons = 0;
+  struct InstrumentedGreater {
+    explicit InstrumentedGreater(std::size_t* cmp) : comparisons_(cmp) {}
+    bool operator()(int rhs, int lhs) const {
+      ++*comparisons_;
+      return std::greater<int>()(rhs, lhs);
+    }
 
-  std::size_t comparisons{};
-  auto cmp = [&comparisons](int rhs, int lhs) {
-    ++comparisons;
-    return std::greater<int>()(rhs, lhs);
+    std::size_t* comparisons_;
   };
+  InstrumentedGreater cmp(&comparisons);
 
   auto i = std::lower_bound(f, l, value, cmp);
   for (auto j = base(f); j != base(i); ++j)
@@ -57,8 +68,10 @@ test(Iter first, Iter last, const T& value)
     assert(!std::greater<int>()(*j, value));
 
   auto len = static_cast<std::size_t>(std::distance(first, last));
+#if TEST_STD_VER > 17
   assert(strides <= 2 * len);
   assert(displacement <= 2 * len);
+#endif
   assert(comparisons <= std::ceil(std::log2(len + 1)));
 }
 

libcxx/test/std/algorithms/alg.sorting/alg.set.operations/set.intersection/ranges_set_intersection.pass.cpp

@@ -436,20 +436,20 @@ constexpr void testComplexityParameterizedIter() {
   }
 
   // Lower complexity when there is low overlap between ranges: we can make 2*log(X) comparisons when one range
-  // has X elements that can be skipped over.
+  // has X elements that can be skipped over (and then 1 more to confirm that the value we found is equal).
   {
     std::array r1{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
     std::array r2{15};
     std::array expected{15};
 
     OperationCounts expectedCounts;
-    expectedCounts.comparisons                 = 8;
-    expectedCounts.in[0].proj                  = 8;
-    expectedCounts.in[0].iterator_strides      = 24;
-    expectedCounts.in[0].iterator_displacement = 24;
-    expectedCounts.in[1].proj                  = 8;
-    expectedCounts.in[1].iterator_strides      = 3;
-    expectedCounts.in[1].iterator_displacement = 3;
+    expectedCounts.comparisons                 = 9;
+    expectedCounts.in[0].proj                  = 9;
+    expectedCounts.in[0].iterator_strides      = 23;
+    expectedCounts.in[0].iterator_displacement = 23;
+    expectedCounts.in[1].proj                  = 9;
+    expectedCounts.in[1].iterator_strides      = 1;
+    expectedCounts.in[1].iterator_displacement = 1;
 
     testSetIntersectionAndReturnOpCounts<In1, In2, Out>(r1, r2, expected, expectedCounts);
     testRangesSetIntersectionAndReturnOpCounts<In1, In2, Out>(r1, r2, expected, expectedCounts);
@@ -721,9 +721,9 @@ constexpr bool test() {
       std::ranges::set_intersection(r1.begin(), r1.end(), r2.begin(), r2.end(), out.data(), comp, proj1, proj2);
 
       assert(std::ranges::equal(out, expected, {}, &Data::data));
-      assert(numberOfComp < maxOperation);
-      assert(numberOfProj1 < maxOperation);
-      assert(numberOfProj2 < maxOperation);
+      assert(numberOfComp <= maxOperation);
+      assert(numberOfProj1 <= maxOperation);
+      assert(numberOfProj2 <= maxOperation);
     }
 
     // range overload