[libcxx-commits] [libcxx] [libc++] `std::ranges::advance`: avoid unneeded bounds checks when advancing iterator (PR #84126)
Jan Kokemüller via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Apr 1 23:57:56 PDT 2024
https://github.com/jiixyj updated https://github.com/llvm/llvm-project/pull/84126
>From 1f82b8f5a573d1c89aa6d3a51a5fb609112b6e36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Wed, 6 Mar 2024 07:49:25 +0100
Subject: [PATCH 01/10] skip unneeded checks against '__bound_sentinel' when
advancing iterator
---
libcxx/include/__iterator/advance.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/include/__iterator/advance.h b/libcxx/include/__iterator/advance.h
index 7959bdeae32643..296db1aaab6526 100644
--- a/libcxx/include/__iterator/advance.h
+++ b/libcxx/include/__iterator/advance.h
@@ -170,14 +170,14 @@ struct __fn {
} else {
// Otherwise, if `n` is non-negative, while `bool(i != bound_sentinel)` is true, increments `i` but at
// most `n` times.
- while (__i != __bound_sentinel && __n > 0) {
+ while (__n > 0 && __i != __bound_sentinel) {
++__i;
--__n;
}
// Otherwise, while `bool(i != bound_sentinel)` is true, decrements `i` but at most `-n` times.
if constexpr (bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>) {
- while (__i != __bound_sentinel && __n < 0) {
+ while (__n < 0 && __i != __bound_sentinel) {
--__i;
++__n;
}
>From 8a2459d38f461ff9716e0006922fe2180994a28a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Sat, 9 Mar 2024 11:22:59 +0100
Subject: [PATCH 02/10] add tests
---
.../iterator_count_sentinel.pass.cpp | 22 ++++++++++++++++++
libcxx/test/support/test_iterators.h | 23 +++++++++++++++++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index a1c15640182162..72584e60d9f89f 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -42,6 +42,13 @@ constexpr void check_forward(int* first, int* last, std::iter_difference_t<It> n
// regardless of the iterator category.
assert(it.stride_count() == M);
assert(it.stride_displacement() == M);
+ if (n == 0) {
+ assert(it.equals_count() == 0);
+ } else {
+ assert(it.equals_count() > 0);
+ assert(it.equals_count() == M || it.equals_count() == M + 1);
+ assert(it.equals_count() <= n);
+ }
}
}
@@ -95,6 +102,7 @@ constexpr void check_backward(int* first, int* last, std::iter_difference_t<It>
(void)std::ranges::advance(it, n, sent);
assert(it.stride_count() <= 1);
assert(it.stride_displacement() <= 1);
+ assert(it.equals_count() == 0);
}
}
@@ -213,6 +221,20 @@ constexpr bool test() {
assert(i == iota_iterator{INT_MIN+1});
}
+ // Check that we don't do an unneeded bounds check when decrementing a
+ // `bidirectional_iterator` that doesn't model `sized_sentinel_for`.
+ {
+ static_assert(std::bidirectional_iterator<bidirectional_iterator<iota_iterator>>);
+ static_assert(!std::sized_sentinel_for<bidirectional_iterator<iota_iterator>,
+ bidirectional_iterator<iota_iterator>>);
+
+ auto it = stride_counting_iterator(bidirectional_iterator(iota_iterator{+1}));
+ auto sent = stride_counting_iterator(bidirectional_iterator(iota_iterator{-2}));
+ assert(std::ranges::advance(it, -3, sent) == 0);
+ assert(base(base(it)) == iota_iterator{-2});
+ assert(it.equals_count() == 3);
+ }
+
return true;
}
diff --git a/libcxx/test/support/test_iterators.h b/libcxx/test/support/test_iterators.h
index c92ce375348ff4..e551ab5a62d08b 100644
--- a/libcxx/test/support/test_iterators.h
+++ b/libcxx/test/support/test_iterators.h
@@ -725,11 +725,14 @@ struct common_input_iterator {
# endif // TEST_STD_VER >= 20
// Iterator adaptor that counts the number of times the iterator has had a successor/predecessor
-// operation called. Has two recorders:
+// operation or an equality comparison operation called. Has three recorders:
// * `stride_count`, which records the total number of calls to an op++, op--, op+=, or op-=.
// * `stride_displacement`, which records the displacement of the calls. This means that both
// op++/op+= will increase the displacement counter by 1, and op--/op-= will decrease the
// displacement counter by 1.
+// * `equals_count`, which records the total number of calls to an op== or op!=. If compared
+// against a sentinel object, that sentinel object must call the `record_equality_comparison`
+// function so that the comparison is counted correctly.
template <class It>
class stride_counting_iterator {
public:
@@ -754,6 +757,8 @@ class stride_counting_iterator {
constexpr difference_type stride_displacement() const { return stride_displacement_; }
+ constexpr difference_type equals_count() const { return equals_count_; }
+
constexpr decltype(auto) operator*() const { return *It(base_); }
constexpr decltype(auto) operator[](difference_type n) const { return It(base_)[n]; }
@@ -838,9 +843,15 @@ class stride_counting_iterator {
return base(x) - base(y);
}
+ constexpr void record_equality_comparison() const
+ {
+ ++equals_count_;
+ }
+
constexpr bool operator==(stride_counting_iterator const& other) const
requires std::sentinel_for<It, It>
{
+ record_equality_comparison();
return It(base_) == It(other.base_);
}
@@ -875,6 +886,7 @@ class stride_counting_iterator {
decltype(base(std::declval<It>())) base_;
difference_type stride_count_ = 0;
difference_type stride_displacement_ = 0;
+ mutable difference_type equals_count_ = 0;
};
template <class It>
stride_counting_iterator(It) -> stride_counting_iterator<It>;
@@ -887,7 +899,14 @@ class sentinel_wrapper {
public:
explicit sentinel_wrapper() = default;
constexpr explicit sentinel_wrapper(const It& it) : base_(base(it)) {}
- constexpr bool operator==(const It& other) const { return base_ == base(other); }
+ constexpr bool operator==(const It& other) const {
+ // If supported, record statistics about the equality operator call
+ // inside `other`.
+ if constexpr (requires { other.record_equality_comparison(); }) {
+ other.record_equality_comparison();
+ }
+ return base_ == base(other);
+ }
friend constexpr It base(const sentinel_wrapper& s) { return It(s.base_); }
private:
decltype(base(std::declval<It>())) base_;
>From e07b579d84de2ff137d697fd19fdf9f84ec577ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Sat, 9 Mar 2024 11:27:49 +0100
Subject: [PATCH 03/10] apply clang-format
---
.../iterator_count_sentinel.pass.cpp | 6 +++---
libcxx/test/support/test_iterators.h | 9 +++------
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 72584e60d9f89f..5dffb5267c391b 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -225,10 +225,10 @@ constexpr bool test() {
// `bidirectional_iterator` that doesn't model `sized_sentinel_for`.
{
static_assert(std::bidirectional_iterator<bidirectional_iterator<iota_iterator>>);
- static_assert(!std::sized_sentinel_for<bidirectional_iterator<iota_iterator>,
- bidirectional_iterator<iota_iterator>>);
+ static_assert(
+ !std::sized_sentinel_for<bidirectional_iterator<iota_iterator>, bidirectional_iterator<iota_iterator>>);
- auto it = stride_counting_iterator(bidirectional_iterator(iota_iterator{+1}));
+ auto it = stride_counting_iterator(bidirectional_iterator(iota_iterator{+1}));
auto sent = stride_counting_iterator(bidirectional_iterator(iota_iterator{-2}));
assert(std::ranges::advance(it, -3, sent) == 0);
assert(base(base(it)) == iota_iterator{-2});
diff --git a/libcxx/test/support/test_iterators.h b/libcxx/test/support/test_iterators.h
index e551ab5a62d08b..7ffb74990fa4dd 100644
--- a/libcxx/test/support/test_iterators.h
+++ b/libcxx/test/support/test_iterators.h
@@ -843,16 +843,13 @@ class stride_counting_iterator {
return base(x) - base(y);
}
- constexpr void record_equality_comparison() const
- {
- ++equals_count_;
- }
+ constexpr void record_equality_comparison() const { ++equals_count_; }
constexpr bool operator==(stride_counting_iterator const& other) const
requires std::sentinel_for<It, It>
{
- record_equality_comparison();
- return It(base_) == It(other.base_);
+ record_equality_comparison();
+ return It(base_) == It(other.base_);
}
friend constexpr bool operator<(stride_counting_iterator const& x, stride_counting_iterator const& y)
>From f23336a1d395e9bf4e8c2cbd3cbd2e25c11a17c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Tue, 19 Mar 2024 19:32:55 +0100
Subject: [PATCH 04/10] remove unneeded check
---
.../range.iter.ops.advance/iterator_count_sentinel.pass.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 5dffb5267c391b..843dcc39d944ca 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -45,7 +45,6 @@ constexpr void check_forward(int* first, int* last, std::iter_difference_t<It> n
if (n == 0) {
assert(it.equals_count() == 0);
} else {
- assert(it.equals_count() > 0);
assert(it.equals_count() == M || it.equals_count() == M + 1);
assert(it.equals_count() <= n);
}
>From 2a3ea3b94117ab55cad98771b675e94ec01b41a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Fri, 29 Mar 2024 09:58:02 +0100
Subject: [PATCH 05/10] test for the extra bounds check iff 'n > M'
---
.../iterator_count_sentinel.pass.cpp | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 843dcc39d944ca..44be579001be13 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -45,7 +45,13 @@ constexpr void check_forward(int* first, int* last, std::iter_difference_t<It> n
if (n == 0) {
assert(it.equals_count() == 0);
} else {
- assert(it.equals_count() == M || it.equals_count() == M + 1);
+ if (n > M) {
+ // We "hit" the bound, so there is one extra equality check.
+ assert(it.equals_count() == M + 1);
+ } else {
+ assert(it.equals_count() == M);
+ }
+ // In any case, there must not be more than `n` bounds checks.
assert(it.equals_count() <= n);
}
}
>From 61c0029158ed82cb204fb651fde6fd74bb1993f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Fri, 29 Mar 2024 10:54:06 +0100
Subject: [PATCH 06/10] adapt 'check_backward' test so that it can test
bidirectional iterators as well
---
.../iterator_count_sentinel.pass.cpp | 59 ++++++++++++-------
1 file changed, 37 insertions(+), 22 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 44be579001be13..c3f378d3af1a30 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -88,7 +88,11 @@ constexpr void check_forward_sized_sentinel(int* first, int* last, std::iter_dif
template <typename It>
constexpr void check_backward(int* first, int* last, std::iter_difference_t<It> n, int* expected) {
- static_assert(std::random_access_iterator<It>, "This test doesn't support non random access iterators");
+ // Check preconditions for `advance` when called with negative `n`:
+ // <https://eel.is/c++draft/iterators#range.iter.op.advance-5>
+ assert(n < 0);
+ static_assert(std::bidirectional_iterator<It>);
+
using Difference = std::iter_difference_t<It>;
Difference const M = (expected - last); // expected travel distance (which is negative)
@@ -104,10 +108,34 @@ constexpr void check_backward(int* first, int* last, std::iter_difference_t<It>
{
auto it = stride_counting_iterator(It(last));
auto sent = stride_counting_iterator(It(first));
+ static_assert(std::bidirectional_iterator<stride_counting_iterator<It>>);
+
(void)std::ranges::advance(it, n, sent);
- assert(it.stride_count() <= 1);
- assert(it.stride_displacement() <= 1);
- assert(it.equals_count() == 0);
+
+ if constexpr (std::sized_sentinel_for<It, It>) {
+ if (expected == first) {
+ // In this case, the algorithm can just do `it = std::move(sent);`
+ // instead of doing iterator arithmetic:
+ // <https://eel.is/c++draft/iterators#range.iter.op.advance-4.1>
+ assert(it.stride_count() == 0);
+ assert(it.stride_displacement() == 0);
+ } else {
+ assert(it.stride_count() == 1);
+ assert(it.stride_displacement() == 1);
+ }
+ assert(it.equals_count() == 0);
+ } else {
+ assert(it.stride_count() == -M);
+ assert(it.stride_displacement() == M);
+ if (-n > -M) {
+ // We "hit" the bound, so there is one extra equality check.
+ assert(it.equals_count() == -M + 1);
+ } else {
+ assert(it.equals_count() == -M);
+ }
+ // In any case, there must not be more than `-n` bounds checks.
+ assert(it.equals_count() <= -n);
+ }
}
}
@@ -201,11 +229,12 @@ constexpr bool test() {
check_forward_sized_sentinel<int*>( range, range+size, n, expected);
}
- {
- // Note that we can only test ranges::advance with a negative n for iterators that
- // are sized sentinels for themselves, because ranges::advance is UB otherwise.
- // In particular, that excludes bidirectional_iterators since those are not sized sentinels.
+ // Exclude the `n == 0` case for the backwards checks.
+ // Input and forward iterators are not tested as the backwards case does
+ // not apply for them.
+ if (n > 0) {
int* expected = n > size ? range : range + size - n;
+ check_backward<bidirectional_iterator<int*>>(range, range+size, -n, expected);
check_backward<random_access_iterator<int*>>(range, range+size, -n, expected);
check_backward<contiguous_iterator<int*>>( range, range+size, -n, expected);
check_backward<int*>( range, range+size, -n, expected);
@@ -226,20 +255,6 @@ constexpr bool test() {
assert(i == iota_iterator{INT_MIN+1});
}
- // Check that we don't do an unneeded bounds check when decrementing a
- // `bidirectional_iterator` that doesn't model `sized_sentinel_for`.
- {
- static_assert(std::bidirectional_iterator<bidirectional_iterator<iota_iterator>>);
- static_assert(
- !std::sized_sentinel_for<bidirectional_iterator<iota_iterator>, bidirectional_iterator<iota_iterator>>);
-
- auto it = stride_counting_iterator(bidirectional_iterator(iota_iterator{+1}));
- auto sent = stride_counting_iterator(bidirectional_iterator(iota_iterator{-2}));
- assert(std::ranges::advance(it, -3, sent) == 0);
- assert(base(base(it)) == iota_iterator{-2});
- assert(it.equals_count() == 3);
- }
-
return true;
}
>From 0be3474814a3ca7637a37129888abffeb01f4e9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Fri, 29 Mar 2024 11:02:05 +0100
Subject: [PATCH 07/10] apply git-clang-format
---
.../range.iter.ops.advance/iterator_count_sentinel.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index c3f378d3af1a30..b989d5e5301627 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -234,7 +234,7 @@ constexpr bool test() {
// not apply for them.
if (n > 0) {
int* expected = n > size ? range : range + size - n;
- check_backward<bidirectional_iterator<int*>>(range, range+size, -n, expected);
+ check_backward<bidirectional_iterator<int*>>(range, range + size, -n, expected);
check_backward<random_access_iterator<int*>>(range, range+size, -n, expected);
check_backward<contiguous_iterator<int*>>( range, range+size, -n, expected);
check_backward<int*>( range, range+size, -n, expected);
>From 28fc7b7d091a9c81278517a6e647cb13ae79fc07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Mon, 1 Apr 2024 12:21:11 +0200
Subject: [PATCH 08/10] simplify test logic by passing in expected results
---
.../iterator_count_sentinel.pass.cpp | 108 +++++++++---------
1 file changed, 55 insertions(+), 53 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index b989d5e5301627..9880c9f5fa2d1e 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -21,9 +21,12 @@
#include "../types.h"
template <bool Count, typename It>
-constexpr void check_forward(int* first, int* last, std::iter_difference_t<It> n, int* expected) {
+constexpr void check_forward(
+ int* first, int* last, std::iter_difference_t<It> n, int* expected, std::ptrdiff_t expected_equals_count) {
using Difference = std::iter_difference_t<It>;
Difference const M = (expected - first); // expected travel distance
+ // `expected_equals_count` is only relevant when `Count` is true.
+ assert(Count || expected_equals_count == -1);
{
It it(first);
@@ -42,18 +45,7 @@ constexpr void check_forward(int* first, int* last, std::iter_difference_t<It> n
// regardless of the iterator category.
assert(it.stride_count() == M);
assert(it.stride_displacement() == M);
- if (n == 0) {
- assert(it.equals_count() == 0);
- } else {
- if (n > M) {
- // We "hit" the bound, so there is one extra equality check.
- assert(it.equals_count() == M + 1);
- } else {
- assert(it.equals_count() == M);
- }
- // In any case, there must not be more than `n` bounds checks.
- assert(it.equals_count() <= n);
- }
+ assert(it.equals_count() == expected_equals_count);
}
}
@@ -86,10 +78,17 @@ constexpr void check_forward_sized_sentinel(int* first, int* last, std::iter_dif
}
}
-template <typename It>
-constexpr void check_backward(int* first, int* last, std::iter_difference_t<It> n, int* expected) {
- // Check preconditions for `advance` when called with negative `n`:
- // <https://eel.is/c++draft/iterators#range.iter.op.advance-5>
+template <bool Count, typename It>
+constexpr void check_backward(
+ int* first,
+ int* last,
+ std::iter_difference_t<It> n,
+ int* expected,
+ std::ptrdiff_t expected_stride_count,
+ std::ptrdiff_t expected_stride_displacement,
+ std::ptrdiff_t expected_equals_count) {
+ // Check preconditions for `advance` when called with negative `n`
+ // (see [range.iter.op.advance]).
assert(n < 0);
static_assert(std::bidirectional_iterator<It>);
@@ -109,33 +108,13 @@ constexpr void check_backward(int* first, int* last, std::iter_difference_t<It>
auto it = stride_counting_iterator(It(last));
auto sent = stride_counting_iterator(It(first));
static_assert(std::bidirectional_iterator<stride_counting_iterator<It>>);
+ static_assert(Count == !std::sized_sentinel_for<It, It>);
(void)std::ranges::advance(it, n, sent);
- if constexpr (std::sized_sentinel_for<It, It>) {
- if (expected == first) {
- // In this case, the algorithm can just do `it = std::move(sent);`
- // instead of doing iterator arithmetic:
- // <https://eel.is/c++draft/iterators#range.iter.op.advance-4.1>
- assert(it.stride_count() == 0);
- assert(it.stride_displacement() == 0);
- } else {
- assert(it.stride_count() == 1);
- assert(it.stride_displacement() == 1);
- }
- assert(it.equals_count() == 0);
- } else {
- assert(it.stride_count() == -M);
- assert(it.stride_displacement() == M);
- if (-n > -M) {
- // We "hit" the bound, so there is one extra equality check.
- assert(it.equals_count() == -M + 1);
- } else {
- assert(it.equals_count() == -M);
- }
- // In any case, there must not be more than `-n` bounds checks.
- assert(it.equals_count() <= -n);
- }
+ assert(it.stride_count() == expected_stride_count);
+ assert(it.stride_displacement() == expected_stride_displacement);
+ assert(it.equals_count() == expected_equals_count);
}
}
@@ -212,13 +191,17 @@ constexpr bool test() {
{
int* expected = n > size ? range + size : range + n;
- check_forward<false, cpp17_input_iterator<int*>>( range, range+size, n, expected);
- check_forward<false, cpp20_input_iterator<int*>>( range, range+size, n, expected);
- check_forward<true, forward_iterator<int*>>( range, range+size, n, expected);
- check_forward<true, bidirectional_iterator<int*>>(range, range+size, n, expected);
- check_forward<true, random_access_iterator<int*>>(range, range+size, n, expected);
- check_forward<true, contiguous_iterator<int*>>( range, range+size, n, expected);
- check_forward<true, int*>( range, range+size, n, expected);
+ std::ptrdiff_t equals_count = n > size ? size + 1 : n;
+
+ // clang-format off
+ check_forward<false, cpp17_input_iterator<int*>>( range, range+size, n, expected, -1);
+ check_forward<false, cpp20_input_iterator<int*>>( range, range+size, n, expected, -1);
+ check_forward<true, forward_iterator<int*>>( range, range+size, n, expected, equals_count);
+ check_forward<true, bidirectional_iterator<int*>>(range, range+size, n, expected, equals_count);
+ check_forward<true, random_access_iterator<int*>>(range, range+size, n, expected, equals_count);
+ check_forward<true, contiguous_iterator<int*>>( range, range+size, n, expected, equals_count);
+ check_forward<true, int*>( range, range+size, n, expected, equals_count);
+ // clang-format on
check_forward_sized_sentinel<cpp17_input_iterator<int*>>( range, range+size, n, expected);
check_forward_sized_sentinel<cpp20_input_iterator<int*>>( range, range+size, n, expected);
@@ -229,15 +212,34 @@ constexpr bool test() {
check_forward_sized_sentinel<int*>( range, range+size, n, expected);
}
- // Exclude the `n == 0` case for the backwards checks.
+ // Exclude the `n == 0` case for the backwards checks (this is tested by
+ // the forward tests above).
// Input and forward iterators are not tested as the backwards case does
// not apply for them.
if (n > 0) {
int* expected = n > size ? range : range + size - n;
- check_backward<bidirectional_iterator<int*>>(range, range + size, -n, expected);
- check_backward<random_access_iterator<int*>>(range, range+size, -n, expected);
- check_backward<contiguous_iterator<int*>>( range, range+size, -n, expected);
- check_backward<int*>( range, range+size, -n, expected);
+ {
+ std::ptrdiff_t stride_count = range + size - expected;
+ std::ptrdiff_t stride_displacement = -stride_count;
+ std::ptrdiff_t equals_count = n > size ? size + 1 : n;
+
+ check_backward<true, bidirectional_iterator<int*>>(
+ range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ }
+ {
+ // If `n >= size`, the algorithm can just do `it = std::move(sent);`
+ // instead of doing iterator arithmetic.
+ std::ptrdiff_t stride_count = (n >= size) ? 0 : 1;
+ std::ptrdiff_t stride_displacement = (n >= size) ? 0 : 1;
+ std::ptrdiff_t equals_count = 0;
+
+ check_backward<false, random_access_iterator<int*>>(
+ range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ check_backward<false, contiguous_iterator<int*>>(
+ range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ check_backward<false, int*>(
+ range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ }
}
}
}
>From 6af34a3748b1307f5bafbbe81a72f6e1815f077d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Tue, 2 Apr 2024 08:48:52 +0200
Subject: [PATCH 09/10] use 'int' instead of 'ptrdiff_t' for expected counts,
wrap them into a struct for 'check_backwards'
---
.../iterator_count_sentinel.pass.cpp | 66 +++++++++----------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 9880c9f5fa2d1e..9a6158f65f7eb1 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -21,8 +21,8 @@
#include "../types.h"
template <bool Count, typename It>
-constexpr void check_forward(
- int* first, int* last, std::iter_difference_t<It> n, int* expected, std::ptrdiff_t expected_equals_count) {
+constexpr void
+check_forward(int* first, int* last, std::iter_difference_t<It> n, int* expected, int expected_equals_count = -1) {
using Difference = std::iter_difference_t<It>;
Difference const M = (expected - first); // expected travel distance
// `expected_equals_count` is only relevant when `Count` is true.
@@ -78,15 +78,15 @@ constexpr void check_forward_sized_sentinel(int* first, int* last, std::iter_dif
}
}
+struct Expected {
+ int stride_count;
+ int stride_displacement;
+ int equals_count;
+};
+
template <bool Count, typename It>
-constexpr void check_backward(
- int* first,
- int* last,
- std::iter_difference_t<It> n,
- int* expected,
- std::ptrdiff_t expected_stride_count,
- std::ptrdiff_t expected_stride_displacement,
- std::ptrdiff_t expected_equals_count) {
+constexpr void
+check_backward(int* first, int* last, std::iter_difference_t<It> n, int* expected, Expected expected_counts) {
// Check preconditions for `advance` when called with negative `n`
// (see [range.iter.op.advance]).
assert(n < 0);
@@ -112,9 +112,9 @@ constexpr void check_backward(
(void)std::ranges::advance(it, n, sent);
- assert(it.stride_count() == expected_stride_count);
- assert(it.stride_displacement() == expected_stride_displacement);
- assert(it.equals_count() == expected_equals_count);
+ assert(it.stride_count() == expected_counts.stride_count);
+ assert(it.stride_displacement() == expected_counts.stride_displacement);
+ assert(it.equals_count() == expected_counts.equals_count);
}
}
@@ -191,11 +191,11 @@ constexpr bool test() {
{
int* expected = n > size ? range + size : range + n;
- std::ptrdiff_t equals_count = n > size ? size + 1 : n;
+ int equals_count = n > size ? size + 1 : n;
// clang-format off
- check_forward<false, cpp17_input_iterator<int*>>( range, range+size, n, expected, -1);
- check_forward<false, cpp20_input_iterator<int*>>( range, range+size, n, expected, -1);
+ check_forward<false, cpp17_input_iterator<int*>>( range, range+size, n, expected);
+ check_forward<false, cpp20_input_iterator<int*>>( range, range+size, n, expected);
check_forward<true, forward_iterator<int*>>( range, range+size, n, expected, equals_count);
check_forward<true, bidirectional_iterator<int*>>(range, range+size, n, expected, equals_count);
check_forward<true, random_access_iterator<int*>>(range, range+size, n, expected, equals_count);
@@ -219,26 +219,26 @@ constexpr bool test() {
if (n > 0) {
int* expected = n > size ? range : range + size - n;
{
- std::ptrdiff_t stride_count = range + size - expected;
- std::ptrdiff_t stride_displacement = -stride_count;
- std::ptrdiff_t equals_count = n > size ? size + 1 : n;
+ Expected expected_counts = {
+ .stride_count = static_cast<int>(range + size - expected),
+ .stride_displacement = -expected_counts.stride_count,
+ .equals_count = n > size ? size + 1 : n,
+ };
- check_backward<true, bidirectional_iterator<int*>>(
- range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ check_backward<true, bidirectional_iterator<int*>>(range, range + size, -n, expected, expected_counts);
}
{
- // If `n >= size`, the algorithm can just do `it = std::move(sent);`
- // instead of doing iterator arithmetic.
- std::ptrdiff_t stride_count = (n >= size) ? 0 : 1;
- std::ptrdiff_t stride_displacement = (n >= size) ? 0 : 1;
- std::ptrdiff_t equals_count = 0;
-
- check_backward<false, random_access_iterator<int*>>(
- range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
- check_backward<false, contiguous_iterator<int*>>(
- range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
- check_backward<false, int*>(
- range, range + size, -n, expected, stride_count, stride_displacement, equals_count);
+ Expected expected_counts = {
+ // If `n >= size`, the algorithm can just do `it = std::move(sent);`
+ // instead of doing iterator arithmetic.
+ .stride_count = (n >= size) ? 0 : 1,
+ .stride_displacement = (n >= size) ? 0 : 1,
+ .equals_count = 0,
+ };
+
+ check_backward<false, random_access_iterator<int*>>(range, range + size, -n, expected, expected_counts);
+ check_backward<false, contiguous_iterator<int*>>(range, range + size, -n, expected, expected_counts);
+ check_backward<false, int*>(range, range + size, -n, expected, expected_counts);
}
}
}
>From 55ef2f08a747af94e77b214b1bb3d7a6476e8d05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Tue, 2 Apr 2024 08:56:10 +0200
Subject: [PATCH 10/10] also test 'n == 0' case in 'check_backward'
---
.../iterator_count_sentinel.pass.cpp | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
index 9a6158f65f7eb1..76439ef93a607a 100644
--- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp
@@ -88,8 +88,8 @@ template <bool Count, typename It>
constexpr void
check_backward(int* first, int* last, std::iter_difference_t<It> n, int* expected, Expected expected_counts) {
// Check preconditions for `advance` when called with negative `n`
- // (see [range.iter.op.advance]).
- assert(n < 0);
+ // (see [range.iter.op.advance]). In addition, allow `n == 0`.
+ assert(n <= 0);
static_assert(std::bidirectional_iterator<It>);
using Difference = std::iter_difference_t<It>;
@@ -212,11 +212,9 @@ constexpr bool test() {
check_forward_sized_sentinel<int*>( range, range+size, n, expected);
}
- // Exclude the `n == 0` case for the backwards checks (this is tested by
- // the forward tests above).
// Input and forward iterators are not tested as the backwards case does
// not apply for them.
- if (n > 0) {
+ {
int* expected = n > size ? range : range + size - n;
{
Expected expected_counts = {
More information about the libcxx-commits
mailing list