Skip to content

Commit 37f9036

Browse files
authored
[libc++] Make drop_view::begin constant time (llvm#72883) (llvm#72929)
As pointed out in llvm#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 llvm#72883
1 parent 516cc98 commit 37f9036

File tree

3 files changed

+76
-1
lines changed

3 files changed

+76
-1
lines changed

libcxx/include/__ranges/drop_view.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ class drop_view : public view_interface<drop_view<_View>> {
9090
_LIBCPP_HIDE_FROM_ABI constexpr auto begin()
9191
requires(!(__simple_view<_View> && random_access_range<const _View> && sized_range<const _View>))
9292
{
93+
if constexpr (random_access_range<_View> && sized_range<_View>) {
94+
const auto __dist = std::min(ranges::distance(__base_), __count_);
95+
return ranges::begin(__base_) + __dist;
96+
}
9397
if constexpr (_UseCache)
9498
if (__cached_begin_.__has_value())
9599
return *__cached_begin_;
@@ -103,7 +107,8 @@ class drop_view : public view_interface<drop_view<_View>> {
103107
_LIBCPP_HIDE_FROM_ABI constexpr auto begin() const
104108
requires random_access_range<const _View> && sized_range<const _View>
105109
{
106-
return ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
110+
const auto __dist = std::min(ranges::distance(__base_), __count_);
111+
return ranges::begin(__base_) + __dist;
107112
}
108113

109114
_LIBCPP_HIDE_FROM_ABI constexpr auto end()

libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,44 @@ constexpr bool test() {
8181

8282
static_assert(!BeginInvocable<const ForwardView>);
8383

84+
{
85+
// non-common non-simple view,
86+
// The wording of the standard is:
87+
// Returns: ranges::next(ranges::begin(base_), count_, ranges::end(base_))
88+
// Note that "Returns" is used here, meaning that we don't have to do it this way.
89+
// In fact, this will use ranges::advance that has O(n) on non-common range.
90+
// but [range.range] requires "amortized constant time" for ranges::begin and ranges::end
91+
// Here, we test that begin() is indeed constant time, by creating a customized
92+
// sentinel and counting how many times the sentinel eq function is called.
93+
// It should be 0 times, but since this test (or any test under libcxx/test/std) is
94+
// also used by other implementations, we relax the condition to that
95+
// sentinel_cmp_calls is a constant number.
96+
int sentinel_cmp_calls_1 = 0;
97+
int sentinel_cmp_calls_2 = 0;
98+
using NonCommonView = MaybeSimpleNonCommonView<false>;
99+
static_assert(std::ranges::random_access_range<NonCommonView>);
100+
static_assert(std::ranges::sized_range<NonCommonView>);
101+
std::ranges::drop_view dropView9_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
102+
std::ranges::drop_view dropView9_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
103+
assert(dropView9_1.begin() == globalBuff + 4);
104+
assert(dropView9_2.begin() == globalBuff + 6);
105+
assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
106+
}
107+
108+
{
109+
// non-common simple view, same as above.
110+
int sentinel_cmp_calls_1 = 0;
111+
int sentinel_cmp_calls_2 = 0;
112+
using NonCommonView = MaybeSimpleNonCommonView<true>;
113+
static_assert(std::ranges::random_access_range<NonCommonView>);
114+
static_assert(std::ranges::sized_range<NonCommonView>);
115+
std::ranges::drop_view dropView10_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
116+
std::ranges::drop_view dropView10_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
117+
assert(dropView10_1.begin() == globalBuff + 4);
118+
assert(dropView10_2.begin() == globalBuff + 6);
119+
assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
120+
}
121+
84122
{
85123
static_assert(std::ranges::random_access_range<const SimpleView>);
86124
static_assert(std::ranges::sized_range<const SimpleView>);

libcxx/test/std/ranges/range.adaptors/range.drop/types.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,38 @@
1414

1515
int globalBuff[8];
1616

17+
template <class T>
18+
struct sentinel {
19+
T* ptr_;
20+
int* num_of_sentinel_cmp_calls;
21+
22+
public:
23+
friend constexpr bool operator==(sentinel const s, T* const ptr) noexcept {
24+
++(*s.num_of_sentinel_cmp_calls);
25+
return {s.ptr_ == ptr};
26+
}
27+
friend constexpr bool operator==(T* const ptr, sentinel const s) noexcept {
28+
++(*s.num_of_sentinel_cmp_calls);
29+
return {s.ptr_ == ptr};
30+
}
31+
friend constexpr bool operator!=(sentinel const s, T* const ptr) noexcept { return !(s == ptr); }
32+
friend constexpr bool operator!=(T* const ptr, sentinel const s) noexcept { return !(s == ptr); }
33+
};
34+
35+
template <bool IsSimple>
36+
struct MaybeSimpleNonCommonView : std::ranges::view_base {
37+
int start_;
38+
int* num_of_sentinel_cmp_calls;
39+
constexpr std::size_t size() const { return 8; }
40+
constexpr int* begin() { return globalBuff + start_; }
41+
constexpr std::conditional_t<IsSimple, int*, const int*> begin() const { return globalBuff + start_; }
42+
constexpr sentinel<int> end() { return sentinel<int>{globalBuff + size(), num_of_sentinel_cmp_calls}; }
43+
constexpr auto end() const {
44+
return std::conditional_t<IsSimple, sentinel<int>, sentinel<const int>>{
45+
globalBuff + size(), num_of_sentinel_cmp_calls};
46+
}
47+
};
48+
1749
struct MoveOnlyView : std::ranges::view_base {
1850
int start_;
1951
constexpr explicit MoveOnlyView(int start = 0) : start_(start) {}

0 commit comments

Comments
 (0)