[libcxx-commits] [libcxx] 37f9036 - [libc++] Make drop_view::begin constant time (#72883) (#72929)

[via libcxx-commits]

Author: Hongyu Ouyang
Date: 2023-12-28T21:27:42Z
New Revision: 37f9036320e37527a58586fb0ba99ca473755574

URL: https://github.com/llvm/llvm-project/commit/37f9036320e37527a58586fb0ba99ca473755574
DIFF: https://github.com/llvm/llvm-project/commit/37f9036320e37527a58586fb0ba99ca473755574.diff

LOG: [libc++] Make drop_view::begin constant time (#72883) (#72929)

As pointed out in #72883, the implementation only needs to return the
value of ranges::next and does not need to obtain the value through
ranges::advance, which causes it to have O(n) complexity in the case
of random-access-sized but non-common range.

Fixes #72883

Added: 
    

Modified: 
    libcxx/include/__ranges/drop_view.h
    libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.drop/types.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index 2b89c6877a7896..83bb598b0a0c91 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -90,6 +90,10 @@ class drop_view : public view_interface<drop_view<_View>> {
   _LIBCPP_HIDE_FROM_ABI constexpr auto begin()
     requires(!(__simple_view<_View> && random_access_range<const _View> && sized_range<const _View>))
   {
+    if constexpr (random_access_range<_View> && sized_range<_View>) {
+      const auto __dist = std::min(ranges::distance(__base_), __count_);
+      return ranges::begin(__base_) + __dist;
+    }
     if constexpr (_UseCache)
       if (__cached_begin_.__has_value())
         return *__cached_begin_;
@@ -103,7 +107,8 @@ class drop_view : public view_interface<drop_view<_View>> {
   _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const
     requires random_access_range<const _View> && sized_range<const _View>
   {
-    return ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
+    const auto __dist = std::min(ranges::distance(__base_), __count_);
+    return ranges::begin(__base_) + __dist;
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr auto end()

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
index cff088453d226e..8c28769acf7fbf 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
@@ -81,6 +81,44 @@ constexpr bool test() {
 
   static_assert(!BeginInvocable<const ForwardView>);
 
+  {
+    // non-common non-simple view,
+    // The wording of the standard is:
+    // Returns: ranges::next(ranges::begin(base_), count_, ranges::end(base_))
+    // Note that "Returns" is used here, meaning that we don't have to do it this way.
+    // In fact, this will use ranges::advance that has O(n) on non-common range.
+    // but [range.range] requires "amortized constant time" for ranges::begin and ranges::end
+    // Here, we test that begin() is indeed constant time, by creating a customized
+    // sentinel and counting how many times the sentinel eq function is called.
+    // It should be 0 times, but since this test (or any test under libcxx/test/std) is
+    // also used by other implementations, we relax the condition to that
+    // sentinel_cmp_calls is a constant number.
+    int sentinel_cmp_calls_1 = 0;
+    int sentinel_cmp_calls_2 = 0;
+    using NonCommonView      = MaybeSimpleNonCommonView<false>;
+    static_assert(std::ranges::random_access_range<NonCommonView>);
+    static_assert(std::ranges::sized_range<NonCommonView>);
+    std::ranges::drop_view dropView9_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
+    std::ranges::drop_view dropView9_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
+    assert(dropView9_1.begin() == globalBuff + 4);
+    assert(dropView9_2.begin() == globalBuff + 6);
+    assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
+  }
+
+  {
+    // non-common simple view, same as above.
+    int sentinel_cmp_calls_1 = 0;
+    int sentinel_cmp_calls_2 = 0;
+    using NonCommonView      = MaybeSimpleNonCommonView<true>;
+    static_assert(std::ranges::random_access_range<NonCommonView>);
+    static_assert(std::ranges::sized_range<NonCommonView>);
+    std::ranges::drop_view dropView10_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
+    std::ranges::drop_view dropView10_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
+    assert(dropView10_1.begin() == globalBuff + 4);
+    assert(dropView10_2.begin() == globalBuff + 6);
+    assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
+  }
+
   {
     static_assert(std::ranges::random_access_range<const SimpleView>);
     static_assert(std::ranges::sized_range<const SimpleView>);

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.drop/types.h b/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
index 32bbddc05ed973..1fc3f05bf5eaab 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
+++ b/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
@@ -14,6 +14,38 @@
 
 int globalBuff[8];
 
+template <class T>
+struct sentinel {
+  T* ptr_;
+  int* num_of_sentinel_cmp_calls;
+
+public:
+  friend constexpr bool operator==(sentinel const s, T* const ptr) noexcept {
+    ++(*s.num_of_sentinel_cmp_calls);
+    return {s.ptr_ == ptr};
+  }
+  friend constexpr bool operator==(T* const ptr, sentinel const s) noexcept {
+    ++(*s.num_of_sentinel_cmp_calls);
+    return {s.ptr_ == ptr};
+  }
+  friend constexpr bool operator!=(sentinel const s, T* const ptr) noexcept { return !(s == ptr); }
+  friend constexpr bool operator!=(T* const ptr, sentinel const s) noexcept { return !(s == ptr); }
+};
+
+template <bool IsSimple>
+struct MaybeSimpleNonCommonView : std::ranges::view_base {
+  int start_;
+  int* num_of_sentinel_cmp_calls;
+  constexpr std::size_t size() const { return 8; }
+  constexpr int* begin() { return globalBuff + start_; }
+  constexpr std::conditional_t<IsSimple, int*, const int*> begin() const { return globalBuff + start_; }
+  constexpr sentinel<int> end() { return sentinel<int>{globalBuff + size(), num_of_sentinel_cmp_calls}; }
+  constexpr auto end() const {
+    return std::conditional_t<IsSimple, sentinel<int>, sentinel<const int>>{
+        globalBuff + size(), num_of_sentinel_cmp_calls};
+  }
+};
+
 struct MoveOnlyView : std::ranges::view_base {
   int start_;
   constexpr explicit MoveOnlyView(int start = 0) : start_(start) {}