This is an archive of the discontinued LLVM Phabricator instance.

Differential D129099

[libcxx][ranges] Create a test tool `ProxyIterator` that customises `iter_move` and `iter_swap`
ClosedPublic

Authored by huixie90 on Jul 4 2022, 5:18 PM.

Details

Reviewers
var-const
philnik
ldionne
Group Reviewers
Restricted Project
Commits
rGa81cc1fc0712: [libcxx][ranges] Create a test tool `ProxyIterator` that customises `iter_move`…
Summary

It is meant to be used in ranges algorithm tests.
It is much simplified version of C++23's tuple + zip_view.
Using std::swap would cause compilation failure and using std::move would not create the correct rvalue proxy which would result in copies.

Diff Detail

Event Timeline

huixie90 created this revision.Jul 4 2022, 5:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2022, 5:18 PM
huixie90 requested review of this revision.Jul 4 2022, 5:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2022, 5:18 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B173609: Diff 442155.Jul 4 2022, 5:34 PM
philnik requested changes to this revision.Jul 5 2022, 12:40 AM

Thanks for working on this! Could you go through the list of already implemented algorithms and instantiate every one of those with at least ProxyIterator<int*>? If there are broken ones, please add the line commented out with a // TODO: Fix proxy iterator above it (to make it easier to grep for). It would also be nice if you could add ProxyIterator<input_iterator<int*>> through ProxyIterator<contiguous_iterator<int*>> to the permutation tests (since we know at least some of them are broken).

libcxx/test/support/test.support/test_proxy.pass.cpp
2

I don't object to ProxyIterator having it's own test, but I wouldn't ask for one either.

18

Use MoveOnly.h?

libcxx/test/support/test_proxy.h
42 ↗(On Diff #442155)

Please don't use implementation details. This makes our tests non-portable.

134 ↗(On Diff #442155)

Should this call base recursively?

161–165 ↗(On Diff #442155)

Can't you just = default this?

266 ↗(On Diff #442155)

Could you add a few static_asserts here to ensure that ProxyIterator meets the correct concepts? (That would be enough testing IMO)

This revision now requires changes to proceed.Jul 5 2022, 12:40 AM
huixie90 added inline comments.Jul 5 2022, 3:19 AM
libcxx/test/support/test_proxy.h
42 ↗(On Diff #442155)

do you have any alternative suggestions for __copy_cvref_t? I don't want to copy the implementations here.

134 ↗(On Diff #442155)

Could you please clarify? I thought my implementation does it recursively. Are you suggesting not to do it recursively?

266 ↗(On Diff #442155)

There are few reason I wrote the test:

  1. It took me few iterations to get all the things correct and having the tests is really helpful for me to make sure the implementation is correct.
  1. This header doesn't depend on those test iterators atm which makes me to put static_assert in the test. (not a big issue though)
philnik added inline comments.Jul 5 2022, 3:42 AM
libcxx/test/support/test_proxy.h
42 ↗(On Diff #442155)

I don't think you have any other option, since there are no standard copy-qualifier type traits and we haven't implemented any for the tests.
Just as a note: this isn't just a theoretical exercise. Microsoft uses the libc++ tests on their own STL implementation (i.e. to detect implementation divergence).

134 ↗(On Diff #442155)

I'm not sure if it should be done recursively. I think we had a discussion on this at some point, but I don't remember the outcome. Looking at the test_iterators it looks like we don't want to call base recursively. I think @ldionne should know that.

266 ↗(On Diff #442155)

Just to make sure: I'm not saying that the test is a bad idea. I just wanted to say that I would consider the test optional.

Since you bring it up: Maybe this should just be in test_iterators.h. I can't really see a case where you'd want this but not the other test iterators and you probably want this in most tests where you want the test iterators (at least going forward).

huixie90 added inline comments.Jul 5 2022, 5:18 AM
libcxx/test/support/test_proxy.h
42 ↗(On Diff #442155)

Thanks for explaining. I will try to find a different approach since the implementation of copy_cvref_t seems a lot of code. One way I can think of is that instead of having this one getData template function, I can have 4 overloads, that takes &, const&, &&, const&&, so that I can specify the cvref manually, which should be less code than copy then implementation. What do you htink

134 ↗(On Diff #442155)

Hmm. I am not so familiar with the design, the reason I call the base of the base is that without doing it, this doesn't work

ProxyIterator<cpp20_input_iterator> == sentinel_wrapper<ProxyIterator<cpp20_input_iterator>>

The reason is, sentinel_wrapper stores the base of the iterator on construction, and compare it with the base of the iterator to be compared.

In this case if the base of the ProxyIterator is cpp20_input_iterator, the above == doesn't work because cpp20_input_iterator cannot be compared with itself.

So I one step further to get the base of the base. I think the major difference is that when we use ProxyIterator, we will have one more level nested iterators than using other test iterators

What do you think?

266 ↗(On Diff #442155)

I will try to move the code into test_iterator

huixie90 updated this revision to Diff 442312.Jul 5 2022, 8:11 AM
huixie90 marked 4 inline comments as done.

addressed review comments

philnik accepted this revision as: philnik.Jul 5 2022, 8:24 AM

LGTM % nits.

libcxx/test/support/test_iterators.h
22–26

I don't think we need to guard the includes here. If you want to avoid depending on transitive includes we should probably add a modulemap. I'll make a PR for that. Sounds like a good idea either way.

844–850

Why aren't these member functions?

Harbormaster completed remote builds in B173713: Diff 442312.Jul 5 2022, 8:31 AM
huixie90 updated this revision to Diff 442330.Jul 5 2022, 9:26 AM

Disable wsign-conversion for one test for now

huixie90 updated this revision to Diff 442336.Jul 5 2022, 9:37 AM
huixie90 marked 3 inline comments as done.

address review comments

Harbormaster completed remote builds in B173727: Diff 442336.Jul 5 2022, 11:32 AM
huixie90 updated this revision to Diff 442374.Jul 5 2022, 12:38 PM

fix CI failure

huixie90 updated this revision to Diff 442381.Jul 5 2022, 1:08 PM

add == and <=> for Proxy (convinient for sort algorithms)

Harbormaster completed remote builds in B173758: Diff 442381.Jul 5 2022, 4:09 PM
var-const requested changes to this revision.Jul 5 2022, 5:25 PM

This is very cool, thank you! I'd like to discuss (and potentially change) how tests are structured before approving.

libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp
130

Rather than adding these checks to every test file, would it be possible to create a dedicated test file, similar to ranges_robust_against_copying_* files? Happy to discuss here or on Discord if you'd like.

libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp
30

Can you please provide more context here? It doesn't seem like either <chrono> or <atomic> (or <ranges>) are included here, and there don't seem to be any changes in includes in any of the standard headers, so why is this an issue now?

(Also: there's an extra 1 character in ADDITIONAL and a _TODO suffix which should probably be removed)

libcxx/test/support/test_iterators.h
830

Can you expand the comment to explain the intention here?

868

This is to make ProxyIterator, not Proxy, indirectly writable, right? If so, can you please make it clear in the comment?

888

These properties (std::indireclty_readable, std::indirectly_writable, anything else that's crucial for these classes to function correctly) should be static_asserted. You can add static assertions right after the class definitions.

903

Nit: s/dereference/dereferenced/.

906

Nit: s/e.g If/E.g. if/.

917

Add line break to place struct on a different line.

948

Nit: move requires to a different line?

1110

Ultranit: opening brace normally goes to the preceding line.

This revision now requires changes to proceed.Jul 5 2022, 5:25 PM
var-const mentioned this in D128744: [libc++][ranges] Implement `ranges::partial_sort`..Jul 5 2022, 8:40 PM
huixie90 added inline comments.Jul 5 2022, 11:35 PM
libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp
130

I also like the idea of a single file, but it is slightly trickier than ranges_robust_against_copying_*. The ranges_robust_against_copying_* test doesn't check runtime behavior. I believe for these tests, we do want to check runtime behavior. For example, for the sorting tests, we do like to check the ranges are sorted. (e.g a triple-move implementation of swap could end up with elements overwritten, and only iter_swap can correctly swap the elements).

But we definitely "can" add a "ranges_robust_against_" test just to test it compiles for all algorithms for proxy iterator. and for individual algorithms that we know they are definitely moving/swapping elements, we can add additional tests to check that the behaviour is correct. what do you think

libcxx/test/support/test_proxy.h
161–165 ↗(On Diff #442155)

I couldn't because it has base class which don't have == defined. I think it is more effort to define the base class's operator== then just spell it out here

huixie90 updated this revision to Diff 442463.Jul 6 2022, 1:31 AM
huixie90 marked 9 inline comments as done.

address review feedback

Harbormaster completed remote builds in B173821: Diff 442463.Jul 6 2022, 3:21 AM
huixie90 updated this revision to Diff 442514.Jul 6 2022, 4:38 AM

CI failure on mac clang

Harbormaster completed remote builds in B173854: Diff 442514.Jul 6 2022, 6:08 AM
var-const accepted this revision as: var-const.Jul 6 2022, 3:01 PM
var-const added inline comments.
libcxx/test/support/test_iterators.h
830

Thanks! The class doesn't have special handling of swap to make sure there's a compilation error if the underlying algorithm uses plain swap, right? If yes, can you also add that to the comment -- perhaps something like "Note that unlike tuple, this class deliberately doesn't have special handling of swap to cause a compilation error if it's used in an algorithm that relies on plain swap instead of ranges::iter_swap".

huixie90 updated this revision to Diff 442698.Jul 6 2022, 3:17 PM
huixie90 marked an inline comment as done.

improve comments

Harbormaster completed remote builds in B173993: Diff 442698.Jul 6 2022, 6:13 PM
philnik accepted this revision.Jul 7 2022, 3:02 PM
This revision is now accepted and ready to land.Jul 7 2022, 3:02 PM
Closed by commit rGa81cc1fc0712: [libcxx][ranges] Create a test tool `ProxyIterator` that customises `iter_move`… (authored by huixie90). · Explain WhyJul 7 2022, 4:15 PM
This revision was automatically updated to reflect the committed changes.
huixie90 added a commit: rGa81cc1fc0712: [libcxx][ranges] Create a test tool `ProxyIterator` that customises `iter_move`….

Revision Contents

PathSize
libcxx/
test/
std/
algorithms/
alg.modifying.operations/
alg.copy/
15 lines
11 lines
24 lines
15 lines
alg.move/
38 lines
35 lines
alg.reverse/
25 lines
alg.swap/
15 lines
alg.sorting/
alg.sort/
sort/
20 lines
stable.sort/
20 lines
iterators/
iterator.primitives/
iterator.operations/
9 lines
support/
test.support/
279 lines
297 lines

Diff 443082

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.pass.cpp

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
constexpr void test_in_iterators() { constexpr void test_in_iterators() {
test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>(); test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>();
test_iterators<forward_iterator<int*>, Out>(); test_iterators<forward_iterator<int*>, Out>();
test_iterators<bidirectional_iterator<int*>, Out>(); test_iterators<bidirectional_iterator<int*>, Out>();
test_iterators<random_access_iterator<int*>, Out>(); test_iterators<random_access_iterator<int*>, Out>();
test_iterators<contiguous_iterator<int*>, Out>(); test_iterators<contiguous_iterator<int*>, Out>();
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<cpp20_input_iterator<int*>>, Out, sentinel_wrapper<ProxyIterator<cpp20_input_iterator<int*>>>>();
test_iterators<ProxyIterator<forward_iterator<int*>>, Out>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
constexpr bool test() { constexpr bool test() {
test_in_iterators<cpp20_input_iterator<int*>>(); test_in_iterators<cpp20_input_iterator<int*>>();
test_in_iterators<forward_iterator<int*>>(); test_in_iterators<forward_iterator<int*>>();
test_in_iterators<bidirectional_iterator<int*>>(); test_in_iterators<bidirectional_iterator<int*>>();
test_in_iterators<random_access_iterator<int*>>(); test_in_iterators<random_access_iterator<int*>>();
test_in_iterators<contiguous_iterator<int*>>(); test_in_iterators<contiguous_iterator<int*>>();
test_proxy_in_iterators<ProxyIterator<cpp20_input_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<forward_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
{ // check that ranges::dangling is returned { // check that ranges::dangling is returned
std::array<int, 4> out; std::array<int, 4> out;
std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret = std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret =
std::ranges::copy(std::array {1, 2, 3, 4}, out.data()); std::ranges::copy(std::array {1, 2, 3, 4}, out.data());
assert(ret.out == out.data() + 4); assert(ret.out == out.data() + 4);
assert((out == std::array{1, 2, 3, 4})); assert((out == std::array{1, 2, 3, 4}));
} }
▲ Show 20 Lines • Show All 87 Lines • Show Last 20 Lines

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_backward.pass.cpp

Show First 20 Lines • Show All 101 Lines • ▼ Show 20 Lines
template <class Out> template <class Out>
constexpr void test_in_iterators() { constexpr void test_in_iterators() {
test_iterators<bidirectional_iterator<int*>, Out>(); test_iterators<bidirectional_iterator<int*>, Out>();
test_iterators<random_access_iterator<int*>, Out>(); test_iterators<random_access_iterator<int*>, Out>();
test_iterators<contiguous_iterator<int*>, Out>(); test_iterators<contiguous_iterator<int*>, Out>();
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
constexpr bool test() { constexpr bool test() {
test_in_iterators<bidirectional_iterator<int*>>(); test_in_iterators<bidirectional_iterator<int*>>();
test_in_iterators<random_access_iterator<int*>>(); test_in_iterators<random_access_iterator<int*>>();
test_in_iterators<contiguous_iterator<int*>>(); test_in_iterators<contiguous_iterator<int*>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
{ // check that ranges::dangling is returned { // check that ranges::dangling is returned
std::array<int, 4> out; std::array<int, 4> out;
std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret = std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret =
std::ranges::copy_backward(std::array {1, 2, 3, 4}, out.data() + out.size()); std::ranges::copy_backward(std::array {1, 2, 3, 4}, out.data() + out.size());
assert(ret.out == out.data()); assert(ret.out == out.data());
assert((out == std::array{1, 2, 3, 4})); assert((out == std::array{1, 2, 3, 4}));
} }
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_if.pass.cpp

Show First 20 Lines • Show All 203 Lines • ▼ Show 20 Lines test_in_iterators<int*>();
int a[] = {5, 4, 3, 2, 1}; int a[] = {5, 4, 3, 2, 1};
int b[5]; int b[5];
std::ranges::copy_if(a, b, pred, proj); std::ranges::copy_if(a, b, pred, proj);
assert(predicateCount == 5); assert(predicateCount == 5);
assert(projectionCount == 5); assert(projectionCount == 5);
} }
} }
{ // test proxy iterator
{
std::array in = {4, 6, 87, 3, 88, 44, 45, 9};
std::array<int, 4> out;
ProxyRange proxyIn{in};
ProxyRange proxyOut{out};
std::ranges::copy_if(proxyIn.begin(), proxyIn.end(), proxyOut.begin(),
[](auto const& i) { return i.data % 2 == 0; });
assert((out == std::array{4, 6, 88, 44}));
}
{
std::array in = {4, 6, 87, 3, 88, 44, 45, 9};
std::array<int, 4> out;
ProxyRange proxyIn{in};
ProxyRange proxyOut{out};
std::ranges::copy_if(proxyIn, proxyOut.begin(), [](const auto& i) { return i.data % 2 == 0; });
assert((out == std::array{4, 6, 88, 44}));
}
}
return true; return true;
} }
int main(int, char**) { int main(int, char**) {
test(); test();
static_assert(test()); static_assert(test());
return 0; return 0;
} }

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_n.pass.cpp

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines
constexpr void test_in_iterators() { constexpr void test_in_iterators() {
test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>(); test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>();
test_iterators<forward_iterator<int*>, Out>(); test_iterators<forward_iterator<int*>, Out>();
test_iterators<bidirectional_iterator<int*>, Out>(); test_iterators<bidirectional_iterator<int*>, Out>();
test_iterators<random_access_iterator<int*>, Out>(); test_iterators<random_access_iterator<int*>, Out>();
test_iterators<contiguous_iterator<int*>, Out>(); test_iterators<contiguous_iterator<int*>, Out>();
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<cpp20_input_iterator<int*>>, Out, sentinel_wrapper<ProxyIterator<cpp20_input_iterator<int*>>>>();
test_iterators<ProxyIterator<forward_iterator<int*>>, Out>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
constexpr bool test() { constexpr bool test() {
test_in_iterators<cpp20_input_iterator<int*>>(); test_in_iterators<cpp20_input_iterator<int*>>();
test_in_iterators<forward_iterator<int*>>(); test_in_iterators<forward_iterator<int*>>();
test_in_iterators<bidirectional_iterator<int*>>(); test_in_iterators<bidirectional_iterator<int*>>();
test_in_iterators<random_access_iterator<int*>>(); test_in_iterators<random_access_iterator<int*>>();
test_in_iterators<contiguous_iterator<int*>>(); test_in_iterators<contiguous_iterator<int*>>();
test_proxy_in_iterators<ProxyIterator<cpp20_input_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<forward_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
{ // check that every element is copied exactly once { // check that every element is copied exactly once
struct CopyOnce { struct CopyOnce {
bool copied = false; bool copied = false;
constexpr CopyOnce() = default; constexpr CopyOnce() = default;
constexpr CopyOnce(const CopyOnce& other) = delete; constexpr CopyOnce(const CopyOnce& other) = delete;
constexpr CopyOnce& operator=(const CopyOnce& other) { constexpr CopyOnce& operator=(const CopyOnce& other) {
assert(!other.copied); assert(!other.copied);
copied = true; copied = true;
Show All 20 Lines

libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Lines
constexpr void test_in_iterators() { constexpr void test_in_iterators() {
test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>(); test_iterators<cpp20_input_iterator<int*>, Out, sentinel_wrapper<cpp20_input_iterator<int*>>>();
test_iterators<forward_iterator<int*>, Out>(); test_iterators<forward_iterator<int*>, Out>();
test_iterators<bidirectional_iterator<int*>, Out>(); test_iterators<bidirectional_iterator<int*>, Out>();
test_iterators<random_access_iterator<int*>, Out>(); test_iterators<random_access_iterator<int*>, Out>();
test_iterators<contiguous_iterator<int*>, Out>(); test_iterators<contiguous_iterator<int*>, Out>();
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<cpp20_input_iterator<int*>>, Out, sentinel_wrapper<ProxyIterator<cpp20_input_iterator<int*>>>>();
test_iterators<ProxyIterator<forward_iterator<int*>>, Out>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
struct IteratorWithMoveIter { struct IteratorWithMoveIter {
using value_type = int; using value_type = int;
using difference_type = int; using difference_type = int;
explicit IteratorWithMoveIter() = default; explicit IteratorWithMoveIter() = default;
int* ptr; int* ptr;
constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {} constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {}
constexpr int& operator*() const; // iterator with iter_move should not be dereferenced constexpr int& operator*() const; // iterator with iter_move should not be dereferenced
Show All 11 Linesconstexpr bool test() {
test_in_iterators<cpp20_output_iterator<int*>>(); test_in_iterators<cpp20_output_iterator<int*>>();
test_in_iterators<cpp17_input_iterator<int*>>(); test_in_iterators<cpp17_input_iterator<int*>>();
test_in_iterators<cpp20_input_iterator<int*>>(); test_in_iterators<cpp20_input_iterator<int*>>();
test_in_iterators<forward_iterator<int*>>(); test_in_iterators<forward_iterator<int*>>();
test_in_iterators<bidirectional_iterator<int*>>(); test_in_iterators<bidirectional_iterator<int*>>();
test_in_iterators<random_access_iterator<int*>>(); test_in_iterators<random_access_iterator<int*>>();
test_in_iterators<contiguous_iterator<int*>>(); test_in_iterators<contiguous_iterator<int*>>();
test_proxy_in_iterators<ProxyIterator<cpp20_input_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<forward_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
{ // check that a move-only type works { // check that a move-only type works
{ {
MoveOnly a[] = {1, 2, 3}; MoveOnly a[] = {1, 2, 3};
MoveOnly b[3]; MoveOnly b[3];
std::ranges::move(a, std::begin(b)); std::ranges::move(a, std::begin(b));
assert(b[0].get() == 1); assert(b[0].get() == 1);
assert(b[1].get() == 2); assert(b[1].get() == 2);
assert(b[2].get() == 3); assert(b[2].get() == 3);
} }
{ {
MoveOnly a[] = {1, 2, 3}; MoveOnly a[] = {1, 2, 3};
MoveOnly b[3]; MoveOnly b[3];
std::ranges::move(std::begin(a), std::end(a), std::begin(b)); std::ranges::move(std::begin(a), std::end(a), std::begin(b));
assert(b[0].get() == 1); assert(b[0].get() == 1);
assert(b[1].get() == 2); assert(b[1].get() == 2);
assert(b[2].get() == 3); assert(b[2].get() == 3);
} }
} }
{ // check that a move-only type works for ProxyIterator
{
MoveOnly a[] = {1, 2, 3};
MoveOnly b[3];
ProxyRange proxyA{a};
ProxyRange proxyB{b};
std::ranges::move(proxyA, std::begin(proxyB));
assert(b[0].get() == 1);
assert(b[1].get() == 2);
assert(b[2].get() == 3);
}
{
MoveOnly a[] = {1, 2, 3};
MoveOnly b[3];
ProxyRange proxyA{a};
ProxyRange proxyB{b};
std::ranges::move(std::begin(proxyA), std::end(proxyA), std::begin(proxyB));
assert(b[0].get() == 1);
assert(b[1].get() == 2);
assert(b[2].get() == 3);
}
}
{ // check that ranges::dangling is returned { // check that ranges::dangling is returned
std::array<int, 4> out; std::array<int, 4> out;
std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> decltype(auto) ret = std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> decltype(auto) ret =
std::ranges::move(std::array {1, 2, 3, 4}, out.data()); std::ranges::move(std::array {1, 2, 3, 4}, out.data());
assert(ret.out == out.data() + 4); assert(ret.out == out.data() + 4);
assert((out == std::array{1, 2, 3, 4})); assert((out == std::array{1, 2, 3, 4}));
} }
▲ Show 20 Lines • Show All 108 Lines • Show Last 20 Lines

libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp

Show First 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
template <class Out> template <class Out>
constexpr void test_in_iterators() { constexpr void test_in_iterators() {
test_iterators<bidirectional_iterator<int*>, Out, sentinel_wrapper<bidirectional_iterator<int*>>>(); test_iterators<bidirectional_iterator<int*>, Out, sentinel_wrapper<bidirectional_iterator<int*>>>();
test_iterators<bidirectional_iterator<int*>, Out>(); test_iterators<bidirectional_iterator<int*>, Out>();
test_iterators<random_access_iterator<int*>, Out>(); test_iterators<random_access_iterator<int*>, Out>();
test_iterators<contiguous_iterator<int*>, Out>(); test_iterators<contiguous_iterator<int*>, Out>();
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out, sentinel_wrapper<ProxyIterator<bidirectional_iterator<int*>>>>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
struct IteratorWithMoveIter { struct IteratorWithMoveIter {
using value_type = int; using value_type = int;
using difference_type = int; using difference_type = int;
explicit IteratorWithMoveIter() = default; explicit IteratorWithMoveIter() = default;
int* ptr; int* ptr;
constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {} constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {}
constexpr int& operator*() const; // iterator with iter_move should not be dereferenced constexpr int& operator*() const; // iterator with iter_move should not be dereferenced
Show All 9 Linesstruct IteratorWithMoveIter {
constexpr bool operator==(const IteratorWithMoveIter& other) const = default; constexpr bool operator==(const IteratorWithMoveIter& other) const = default;
}; };
constexpr bool test() { constexpr bool test() {
test_in_iterators<bidirectional_iterator<int*>>(); test_in_iterators<bidirectional_iterator<int*>>();
test_in_iterators<random_access_iterator<int*>>(); test_in_iterators<random_access_iterator<int*>>();
test_in_iterators<contiguous_iterator<int*>>(); test_in_iterators<contiguous_iterator<int*>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
var-constUnsubmitted
Not Done
ReplyInline Actions

Rather than adding these checks to every test file, would it be possible to create a dedicated test file, similar to ranges_robust_against_copying_* files? Happy to discuss here or on Discord if you'd like.

var-const: Rather than adding these checks to every test file, would it be possible to create a dedicated…
huixie90AuthorUnsubmitted
Done
ReplyInline Actions

I also like the idea of a single file, but it is slightly trickier than ranges_robust_against_copying_*. The ranges_robust_against_copying_* test doesn't check runtime behavior. I believe for these tests, we do want to check runtime behavior. For example, for the sorting tests, we do like to check the ranges are sorted. (e.g a triple-move implementation of swap could end up with elements overwritten, and only iter_swap can correctly swap the elements).

But we definitely "can" add a "ranges_robust_against_" test just to test it compiles for all algorithms for proxy iterator. and for individual algorithms that we know they are definitely moving/swapping elements, we can add additional tests to check that the behaviour is correct. what do you think

huixie90: I also like the idea of a single file, but it is slightly trickier than…
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
{ // check that a move-only type works { // check that a move-only type works
{ {
MoveOnly a[] = {1, 2, 3}; MoveOnly a[] = {1, 2, 3};
MoveOnly b[3]; MoveOnly b[3];
std::ranges::move_backward(a, std::end(b)); std::ranges::move_backward(a, std::end(b));
assert(b[0].get() == 1); assert(b[0].get() == 1);
assert(b[1].get() == 2); assert(b[1].get() == 2);
assert(b[2].get() == 3); assert(b[2].get() == 3);
} }
{ {
MoveOnly a[] = {1, 2, 3}; MoveOnly a[] = {1, 2, 3};
MoveOnly b[3]; MoveOnly b[3];
std::ranges::move_backward(std::begin(a), std::end(a), std::end(b)); std::ranges::move_backward(std::begin(a), std::end(a), std::end(b));
assert(b[0].get() == 1); assert(b[0].get() == 1);
assert(b[1].get() == 2); assert(b[1].get() == 2);
assert(b[2].get() == 3); assert(b[2].get() == 3);
} }
} }
{ // check that a move-only type works for ProxyIterator
{
MoveOnly a[] = {1, 2, 3};
MoveOnly b[3];
ProxyRange proxyA{a};
ProxyRange proxyB{b};
std::ranges::move_backward(proxyA, std::ranges::next(proxyB.begin(), std::end(proxyB)));
assert(b[0].get() == 1);
assert(b[1].get() == 2);
assert(b[2].get() == 3);
}
{
MoveOnly a[] = {1, 2, 3};
MoveOnly b[3];
ProxyRange proxyA{a};
ProxyRange proxyB{b};
std::ranges::move_backward(std::begin(proxyA), std::end(proxyA), std::ranges::next(proxyB.begin(), std::end(proxyB)));
assert(b[0].get() == 1);
assert(b[1].get() == 2);
assert(b[2].get() == 3);
}
}
{ // check that ranges::dangling is returned { // check that ranges::dangling is returned
std::array<int, 4> out; std::array<int, 4> out;
std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret = std::same_as<std::ranges::in_out_result<std::ranges::dangling, int*>> auto ret =
std::ranges::move_backward(std::array {1, 2, 3, 4}, out.data() + out.size()); std::ranges::move_backward(std::array {1, 2, 3, 4}, out.data() + out.size());
assert(ret.out == out.data()); assert(ret.out == out.data());
assert((out == std::array{1, 2, 3, 4})); assert((out == std::array{1, 2, 3, 4}));
} }
▲ Show 20 Lines • Show All 108 Lines • Show Last 20 Lines

libcxx/test/std/algorithms/alg.modifying.operations/alg.reverse/ranges.reverse.pass.cpp

Show All 18 Lines
// constexpr borrowed_iterator_t<R> ranges::reverse(R&& r); // constexpr borrowed_iterator_t<R> ranges::reverse(R&& r);
#include <algorithm> #include <algorithm>
#include <array> #include <array>
#include <concepts> #include <concepts>
#include <ranges> #include <ranges>
#include "almost_satisfies_types.h" #include "almost_satisfies_types.h"
#include "MoveOnly.h"
#include "test_iterators.h" #include "test_iterators.h"
template <class Iter, class Sent = sentinel_wrapper<Iter>> template <class Iter, class Sent = sentinel_wrapper<Iter>>
concept HasReverseIt = requires (Iter first, Sent last) { std::ranges::reverse(first, last); }; concept HasReverseIt = requires (Iter first, Sent last) { std::ranges::reverse(first, last); };
static_assert(HasReverseIt<int*>); static_assert(HasReverseIt<int*>);
static_assert(!HasReverseIt<BidirectionalIteratorNotDerivedFrom>); static_assert(!HasReverseIt<BidirectionalIteratorNotDerivedFrom>);
static_assert(!HasReverseIt<BidirectionalIteratorNotDecrementable>); static_assert(!HasReverseIt<BidirectionalIteratorNotDecrementable>);
▲ Show 20 Lines • Show All 49 Lines • ▼ Show 20 Linesconstexpr bool test() {
test_iterators<bidirectional_iterator<int*>>(); test_iterators<bidirectional_iterator<int*>>();
test_iterators<bidirectional_iterator<int*>, sentinel_wrapper<bidirectional_iterator<int*>>>(); test_iterators<bidirectional_iterator<int*>, sentinel_wrapper<bidirectional_iterator<int*>>>();
test_iterators<random_access_iterator<int*>>(); test_iterators<random_access_iterator<int*>>();
test_iterators<random_access_iterator<int*>, sentinel_wrapper<random_access_iterator<int*>>>(); test_iterators<random_access_iterator<int*>, sentinel_wrapper<random_access_iterator<int*>>>();
test_iterators<contiguous_iterator<int*>>(); test_iterators<contiguous_iterator<int*>>();
test_iterators<contiguous_iterator<int*>, sentinel_wrapper<contiguous_iterator<int*>>>(); test_iterators<contiguous_iterator<int*>, sentinel_wrapper<contiguous_iterator<int*>>>();
test_iterators<int*>(); test_iterators<int*>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>>();
// check that std::ranges::dangling is returned // check that std::ranges::dangling is returned
{ {
[[maybe_unused]] std::same_as<std::ranges::dangling> auto ret = std::ranges::reverse(std::array {1, 2, 3, 4}); [[maybe_unused]] std::same_as<std::ranges::dangling> auto ret = std::ranges::reverse(std::array {1, 2, 3, 4});
} }
{ {
{ {
int counter = 0; int counter = 0;
SwapCounter a[] = {&counter, &counter, &counter, &counter}; SwapCounter a[] = {&counter, &counter, &counter, &counter};
std::ranges::reverse(a); std::ranges::reverse(a);
assert(counter == 2); assert(counter == 2);
} }
{ {
int counter = 0; int counter = 0;
SwapCounter a[] = {&counter, &counter, &counter, &counter}; SwapCounter a[] = {&counter, &counter, &counter, &counter};
std::ranges::reverse(a, a + 4); std::ranges::reverse(a, a + 4);
assert(counter == 2); assert(counter == 2);
} }
} }
// Move only types work for ProxyIterator
{
{
MoveOnly a[] = {1, 2, 3};
ProxyRange proxyA{a};
std::ranges::reverse(proxyA.begin(), proxyA.end());
assert(a[0].get() == 3);
assert(a[1].get() == 2);
assert(a[2].get() == 1);
}
{
MoveOnly a[] = {1, 2, 3};
ProxyRange proxyA{a};
std::ranges::reverse(proxyA);
assert(a[0].get() == 3);
assert(a[1].get() == 2);
assert(a[2].get() == 1);
}
}
return true; return true;
} }
int main(int, char**) { int main(int, char**) {
test(); test();
static_assert(test()); static_assert(test());
return 0; return 0;
} }

libcxx/test/std/algorithms/alg.modifying.operations/alg.swap/ranges.swap_ranges.pass.cpp

Show First 20 Lines • Show All 156 Lines • ▼ Show 20 Linesconstexpr void test_rval_range() {
std::array<int, 3> r = {1, 2, 3}; std::array<int, 3> r = {1, 2, 3};
using Expected = std::ranges::swap_ranges_result<std::ranges::dangling, int*>; using Expected = std::ranges::swap_ranges_result<std::ranges::dangling, int*>;
std::same_as<Expected> auto b = std::ranges::swap_ranges(std::array{4, 5, 6}, r); std::same_as<Expected> auto b = std::ranges::swap_ranges(std::array{4, 5, 6}, r);
assert((r == std::array{4, 5, 6})); assert((r == std::array{4, 5, 6}));
assert(b.in2 == r.begin() + 3); assert(b.in2 == r.begin() + 3);
} }
} }
template <class Out>
constexpr void test_proxy_in_iterators() {
test_iterators<ProxyIterator<cpp20_input_iterator<int*>>, Out>();
test_iterators<ProxyIterator<forward_iterator<int*>>, Out>();
test_iterators<ProxyIterator<bidirectional_iterator<int*>>, Out>();
test_iterators<ProxyIterator<random_access_iterator<int*>>, Out>();
test_iterators<ProxyIterator<contiguous_iterator<int*>>, Out>();
}
constexpr bool test() { constexpr bool test() {
test_range(); test_range();
test_iterators<cpp20_input_iterator<int*>, cpp20_input_iterator<int*>>(); test_iterators<cpp20_input_iterator<int*>, cpp20_input_iterator<int*>>();
test_iterators<cpp20_input_iterator<int*>, forward_iterator<int*>>(); test_iterators<cpp20_input_iterator<int*>, forward_iterator<int*>>();
test_iterators<cpp20_input_iterator<int*>, bidirectional_iterator<int*>>(); test_iterators<cpp20_input_iterator<int*>, bidirectional_iterator<int*>>();
test_iterators<cpp20_input_iterator<int*>, random_access_iterator<int*>>(); test_iterators<cpp20_input_iterator<int*>, random_access_iterator<int*>>();
test_iterators<cpp20_input_iterator<int*>, int*>(); test_iterators<cpp20_input_iterator<int*>, int*>();
Show All 17 Linesconstexpr bool test() {
test_iterators<random_access_iterator<int*>, int*>(); test_iterators<random_access_iterator<int*>, int*>();
test_iterators<int*, cpp20_input_iterator<int*>>(); test_iterators<int*, cpp20_input_iterator<int*>>();
test_iterators<int*, forward_iterator<int*>>(); test_iterators<int*, forward_iterator<int*>>();
test_iterators<int*, bidirectional_iterator<int*>>(); test_iterators<int*, bidirectional_iterator<int*>>();
test_iterators<int*, random_access_iterator<int*>>(); test_iterators<int*, random_access_iterator<int*>>();
test_iterators<int*, int*>(); test_iterators<int*, int*>();
test_proxy_in_iterators<ProxyIterator<cpp20_input_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<forward_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<bidirectional_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<random_access_iterator<int*>>>();
test_proxy_in_iterators<ProxyIterator<contiguous_iterator<int*>>>();
test_sentinel(); test_sentinel();
test_different_lengths(); test_different_lengths();
test_borrowed_input_range(); test_borrowed_input_range();
test_rval_range(); test_rval_range();
return true; return true;
} }
static_assert(std::same_as<std::ranges::swap_ranges_result<int, char>, std::ranges::in_in_result<int, char>>); static_assert(std::same_as<std::ranges::swap_ranges_result<int, char>, std::ranges::in_in_result<int, char>>);
int main(int, char**) { int main(int, char**) {
test(); test();
static_assert(test()); static_assert(test());
return 0; return 0;
} }

libcxx/test/std/algorithms/alg.sorting/alg.sort/sort/ranges.sort.pass.cpp

Show First 20 Lines • Show All 199 Lines • ▼ Show 20 Lines struct S {
assert(last == in.end()); assert(last == in.end());
} }
} }
{ // `std::ranges::dangling` is returned. { // `std::ranges::dangling` is returned.
[[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) result = std::ranges::sort(std::array{1, 2, 3}); [[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) result = std::ranges::sort(std::array{1, 2, 3});
} }
// TODO: Enable the tests once the implementation switched to use iter_move/iter_swap
/*
{ // ProxyIterator
{
std::array in = {2, 1, 3};
ProxyRange proxy{in};
std::ranges::sort(proxy.begin(), proxy.end(), [](auto i, auto j) { return i.data < j.data; });
assert((in == std::array{1, 2, 3}));
}
{
std::array in = {2, 1, 3};
ProxyRange proxy{in};
std::ranges::sort(proxy, [](auto i, auto j) { return i.data < j.data; });
assert((in == std::array{1, 2, 3}));
}
}
*/
return true; return true;
} }
int main(int, char**) { int main(int, char**) {
test(); test();
static_assert(test()); static_assert(test());
return 0; return 0;
} }

libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/ranges.stable.sort.pass.cpp

Show First 20 Lines • Show All 253 Lines • ▼ Show 20 Lines struct S {
assert(last == in.end()); assert(last == in.end());
} }
} }
{ // `std::ranges::dangling` is returned. { // `std::ranges::dangling` is returned.
[[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) result = [[maybe_unused]] std::same_as<std::ranges::dangling> decltype(auto) result =
std::ranges::stable_sort(std::array{1, 2, 3}); std::ranges::stable_sort(std::array{1, 2, 3});
} }
// TODO: Enable the tests once the implementation switched to use iter_move/iter_swap
/*
{ // ProxyIterator
{
std::array in = {2, 1, 3};
ProxyRange proxy{in};
std::ranges::stable_sort(proxy.begin(), proxy.end(), [](auto i, auto j) { return i.data < j.data; });
assert((in == std::array{1, 2, 3}));
}
{
std::array in = {2, 1, 3};
ProxyRange proxy{in};
std::ranges::stable_sort(proxy, [](auto i, auto j) { return i.data < j.data; });
assert((in == std::array{1, 2, 3}));
}
}
*/
} }
int main(int, char**) { int main(int, char**) {
test(); test();
// Note: `stable_sort` is not `constexpr`. // Note: `stable_sort` is not `constexpr`.
return 0; return 0;
} }

libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp

Show All 13 Lines
// constexpr void advance(Iter& i, Distance n); // constexpr void advance(Iter& i, Distance n);
// //
// template <BidirectionalIterator Iter, class Distance> // template <BidirectionalIterator Iter, class Distance>
// constexpr void advance(Iter& i, Distance n); // constexpr void advance(Iter& i, Distance n);
// //
// template <RandomAccessIterator Iter, class Distance> // template <RandomAccessIterator Iter, class Distance>
// constexpr void advance(Iter& i, Distance n); // constexpr void advance(Iter& i, Distance n);
// TODO: test_iterators.h includes <ranges>, and <ranges> includes <chrono> and <atomic>.
// Lots of implementation headers under <__chrono/> and <atomic> has signed to unsigned conversion,
// which will trigger the -Wsign-conversion warning.
// Once those headers are fixed, enable the -Wsign-conversion for this test by removing
// <TODO:Remove brackets> below
// Make sure we catch forced conversions to the difference_type if they happen. // Make sure we catch forced conversions to the difference_type if they happen.
// ADDITIONAL_COMPILE_FLAGS: -Wsign-conversion // ADDITIONAL_COMPILE_FLAGS<TODO:Remove brackets>: -Wsign-conversion
var-constUnsubmitted
Done
ReplyInline Actions

Can you please provide more context here? It doesn't seem like either <chrono> or <atomic> (or <ranges>) are included here, and there don't seem to be any changes in includes in any of the standard headers, so why is this an issue now?

(Also: there's an extra 1 character in ADDITIONAL and a _TODO suffix which should probably be removed)

var-const: Can you please provide more context here? It doesn't seem like either `<chrono>` or `<atomic>`…
#include <iterator> #include <iterator>
#include <cassert> #include <cassert>
#include <cstddef> #include <cstddef>
#include <type_traits> #include <type_traits>
#include "test_macros.h" #include "test_macros.h"
#include "test_iterators.h" #include "test_iterators.h"
▲ Show 20 Lines • Show All 56 Lines • Show Last 20 Lines

libcxx/test/support/test.support/test_proxy.pass.cpp

  • This file was added.
//===----------------------------------------------------------------------===//
//
philnikUnsubmitted
Done
ReplyInline Actions

I don't object to ProxyIterator having it's own test, but I wouldn't ask for one either.

philnik: I don't object to `ProxyIterator` having it's own test, but I wouldn't ask for one either.
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// UNSUPPORTED: c++03, c++11, c++14, c++17
// UNSUPPORTED: libcpp-has-no-incomplete-ranges
#include "MoveOnly.h"
#include "test_iterators.h"
#include <cassert>
constexpr void testProxy() {
// constructor value
philnikUnsubmitted
Done
ReplyInline Actions

Use MoveOnly.h?

philnik: Use `MoveOnly.h`?
{
Proxy<int> p{5};
assert(p.data == 5);
}
// constructor reference
{
int i = 5;
Proxy<int&> p{i};
assert(&p.data == &i);
}
// constructor conversion
{
int i = 5;
Proxy<int&> p1{i};
Proxy<int> p2 = p1;
assert(p2.data == 5);
Proxy<int&> p3{p2};
assert(&(p3.data) == &(p2.data));
MoveOnly m1{8};
Proxy<MoveOnly&&> p4 = std::move(m1);
Proxy<MoveOnly> p5 = std::move(p4);
assert(p5.data.get() == 8);
}
// assignment
{
Proxy<int> p1{5};
Proxy<int> p2{6};
p1 = p2;
assert(p1.data == 6);
MoveOnly m1{8};
Proxy<MoveOnly&&> p3 = std::move(m1);
Proxy<MoveOnly> p4{MoveOnly{9}};
p4 = std::move(p3);
assert(p4.data.get() == 8);
int i = 5, j = 6;
Proxy<int&> p5{i};
p5 = Proxy<int&>{j};
assert(p5.data == 6);
}
// const assignment
{
int i = 5;
int j = 6;
const Proxy<int&> p1{i};
const Proxy<int&> p2{j};
p1 = p2;
assert(i == 6);
MoveOnly m1{8};
MoveOnly m2{9};
Proxy<MoveOnly&&> p3 = std::move(m1);
const Proxy<MoveOnly&&> p4 = std::move(m2);
p4 = std::move(p3);
assert(p4.data.get() == 8);
}
// compare
{
Proxy<int> p1{5};
Proxy<int> p2{6};
assert(p1 != p2);
assert(p1 < p2);
}
}
static_assert(std::input_iterator<ProxyIterator<cpp20_input_iterator<int*>>>);
static_assert(!std::forward_iterator<ProxyIterator<cpp20_input_iterator<int*>>>);
static_assert(std::forward_iterator<ProxyIterator<forward_iterator<int*>>>);
static_assert(!std::bidirectional_iterator<ProxyIterator<forward_iterator<int*>>>);
static_assert(std::bidirectional_iterator<ProxyIterator<bidirectional_iterator<int*>>>);
static_assert(!std::random_access_iterator<ProxyIterator<bidirectional_iterator<int*>>>);
static_assert(std::random_access_iterator<ProxyIterator<random_access_iterator<int*>>>);
static_assert(!std::contiguous_iterator<ProxyIterator<random_access_iterator<int*>>>);
static_assert(std::random_access_iterator<ProxyIterator<contiguous_iterator<int*>>>);
static_assert(!std::contiguous_iterator<ProxyIterator<contiguous_iterator<int*>>>);
template <class Iter>
constexpr void testInputIteratorOperation() {
int data[] = {1, 2};
ProxyIterator<Iter> iter{Iter{data}};
sentinel_wrapper<ProxyIterator<Iter>> sent{ProxyIterator<Iter>{Iter{data + 2}}};
std::same_as<Proxy<int&>> decltype(auto) result = *iter;
assert(result.data == 1);
auto& iter2 = ++iter;
static_assert(std::is_same_v<decltype(++iter), ProxyIterator<Iter>&>);
assert(&iter2 == &iter);
assert((*iter).data == 2);
++iter;
assert(iter == sent);
}
template <class Iter>
constexpr void testForwardIteratorOperation() {
int data[] = {1, 2};
ProxyIterator<Iter> iter{Iter{data}};
std::same_as<ProxyIterator<Iter>> decltype(auto) it2 = iter++;
assert((*it2).data == 1);
assert((*iter).data == 2);
}
template <class Iter>
constexpr void testBidirectionalIteratorOperation() {
int data[] = {1, 2};
ProxyIterator<Iter> iter{Iter{data}};
++iter;
assert((*iter).data == 2);
auto& iter2 = --iter;
static_assert(std::is_same_v<decltype(--iter), ProxyIterator<Iter>&>);
assert(&iter2 == &iter);
assert((*iter).data == 1);
++iter;
std::same_as<ProxyIterator<Iter>> decltype(auto) iter3 = iter--;
assert((*iter).data == 1);
assert((*iter3).data == 2);
}
template <class Iter>
constexpr void testRandomAccessIteratorOperation() {
int data[] = {1, 2, 3, 4, 5};
ProxyIterator<Iter> iter{Iter{data}};
auto& iter2 = iter += 2;
static_assert(std::is_same_v<decltype(iter += 2), ProxyIterator<Iter>&>);
assert(&iter2 == &iter);
assert((*iter).data == 3);
auto& iter3 = iter -= 1;
static_assert(std::is_same_v<decltype(iter -= 1), ProxyIterator<Iter>&>);
assert(&iter3 == &iter);
assert((*iter).data == 2);
std::same_as<Proxy<int&>> decltype(auto) r = iter[2];
assert(r.data == 4);
std::same_as<ProxyIterator<Iter>> decltype(auto) iter4 = iter - 1;
assert((*iter4).data == 1);
std::same_as<ProxyIterator<Iter>> decltype(auto) iter5 = iter4 + 2;
assert((*iter5).data == 3);
std::same_as<ProxyIterator<Iter>> decltype(auto) iter6 = 3 + iter4;
assert((*iter6).data == 4);
std::same_as<std::iter_difference_t<Iter>> decltype(auto) n = iter6 - iter5;
assert(n == 1);
assert(iter4 < iter5);
assert(iter3 <= iter5);
assert(iter5 > iter4);
assert(iter6 >= iter4);
}
constexpr void testProxyIterator() {
// input iterator operations
{
testInputIteratorOperation<cpp20_input_iterator<int*>>();
testInputIteratorOperation<forward_iterator<int*>>();
testInputIteratorOperation<bidirectional_iterator<int*>>();
testInputIteratorOperation<random_access_iterator<int*>>();
testInputIteratorOperation<contiguous_iterator<int*>>();
}
// forward iterator operations
{
testForwardIteratorOperation<forward_iterator<int*>>();
testForwardIteratorOperation<bidirectional_iterator<int*>>();
testForwardIteratorOperation<random_access_iterator<int*>>();
testForwardIteratorOperation<contiguous_iterator<int*>>();
}
// bidirectional iterator operations
{
testBidirectionalIteratorOperation<bidirectional_iterator<int*>>();
testBidirectionalIteratorOperation<random_access_iterator<int*>>();
testBidirectionalIteratorOperation<contiguous_iterator<int*>>();
}
// random access iterator operations
{
testRandomAccessIteratorOperation<random_access_iterator<int*>>();
testRandomAccessIteratorOperation<contiguous_iterator<int*>>();
}
}
constexpr void testProxyRange() {
int data[] = {3, 4, 5};
ProxyRange r{data};
std::same_as<ProxyIterator<int*>> decltype(auto) it = std::ranges::begin(r);
assert((*it).data == 3);
it += 3;
assert(it == std::ranges::end(r));
}
template <class Iter>
concept StdMoveWorks = requires(std::iter_value_t<Iter> val, Iter iter) { val = std::move(*iter); };
static_assert(StdMoveWorks<MoveOnly*>);
static_assert(!StdMoveWorks<ProxyIterator<MoveOnly*>>);
// although this "works" but it actually creates a copy instead of move
static_assert(StdMoveWorks<ProxyIterator<int*>>);
using std::swap;
template <class Iter>
concept SwapWorks = requires(Iter iter1, Iter iter2) { swap(*iter1, *iter2); };
static_assert(SwapWorks<int*>);
static_assert(!SwapWorks<ProxyIterator<int*>>);
constexpr bool test() {
testProxy();
testProxyIterator();
testProxyRange();
// iter_move
{
MoveOnly data[] = {5, 6, 7};
ProxyRange r{data};
auto it = r.begin();
std::iter_value_t<decltype(it)> moved = std::ranges::iter_move(it);
assert(moved.data.get() == 5);
}
// iter_swap
{
MoveOnly data[] = {5, 6, 7};
ProxyRange r{data};
auto it1 = r.begin();
auto it2 = it1 + 2;
std::ranges::iter_swap(it1, it2);
assert(data[0].get() == 7);
assert(data[2].get() == 5);
}
return true;
}
int main(int, const char**) {
test();
static_assert(test());
return 0;
}

libcxx/test/support/test_iterators.h

//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef SUPPORT_TEST_ITERATORS_H #ifndef SUPPORT_TEST_ITERATORS_H
#define SUPPORT_TEST_ITERATORS_H #define SUPPORT_TEST_ITERATORS_H
#include <cassert> #include <cassert>
#include <concepts> #include <concepts>
#include <iterator> #include <iterator>
#include <ranges>
#include <stdexcept> #include <stdexcept>
#include <type_traits>
#include <utility> #include <utility>
#include "test_macros.h" #include "test_macros.h"
// This iterator meets C++20's Cpp17OutputIterator requirements, as described // This iterator meets C++20's Cpp17OutputIterator requirements, as described
// in Table 90 ([output.iterators]). // in Table 90 ([output.iterators]).
template <class It> template <class It>
class cpp17_output_iterator class cpp17_output_iterator
philnikUnsubmitted
Done
ReplyInline Actions

I don't think we need to guard the includes here. If you want to avoid depending on transitive includes we should probably add a modulemap. I'll make a PR for that. Sounds like a good idea either way.

philnik: I don't think we need to guard the includes here. If you want to avoid depending on transitive…
{ {
It it_; It it_;
template <class U> friend class cpp17_output_iterator; template <class U> friend class cpp17_output_iterator;
public: public:
typedef std::output_iterator_tag iterator_category; typedef std::output_iterator_tag iterator_category;
typedef void value_type; typedef void value_type;
typedef typename std::iterator_traits<It>::difference_type difference_type; typedef typename std::iterator_traits<It>::difference_type difference_type;
▲ Show 20 Lines • Show All 783 Lines • ▼ Show 20 Lines constexpr friend value_type&& iter_move(Iterator iter) {
return std::move(*iter); return std::move(*iter);
} }
constexpr friend bool operator==(const Iterator& lhs, const Iterator& rhs) { return lhs.ptr_ == rhs.ptr_; } constexpr friend bool operator==(const Iterator& lhs, const Iterator& rhs) { return lhs.ptr_ == rhs.ptr_; }
}; };
} // namespace adl } // namespace adl
// Proxy
// ======================================================================
// Proxy that can wrap a value or a reference. It simulates C++23's tuple
// but simplified to just hold one argument.
// Note that unlike tuple, this class deliberately doesn't have special handling
var-constUnsubmitted
Done
ReplyInline Actions

Can you expand the comment to explain the intention here?

var-const: Can you expand the comment to explain the intention here?
var-constUnsubmitted
Done
ReplyInline Actions

Thanks! The class doesn't have special handling of swap to make sure there's a compilation error if the underlying algorithm uses plain swap, right? If yes, can you also add that to the comment -- perhaps something like "Note that unlike tuple, this class deliberately doesn't have special handling of swap to cause a compilation error if it's used in an algorithm that relies on plain swap instead of ranges::iter_swap".

var-const: Thanks! The class doesn't have special handling of `swap` to make sure there's a compilation…
// of swap to cause a compilation error if it's used in an algorithm that relies
// on plain swap instead of ranges::iter_swap.
// This class is useful for testing that if algorithms support proxy iterator
// properly, i.e. calling ranges::iter_swap and ranges::iter_move instead of
// plain swap and std::move
template <class T>
struct Proxy;
template <class T>
inline constexpr bool IsProxy = false;
template <class T>
inline constexpr bool IsProxy<Proxy<T>> = true;
template <class T>
struct Proxy {
T data;
constexpr T& getData() & { return data; }
philnikUnsubmitted
Done
ReplyInline Actions

Why aren't these member functions?

philnik: Why aren't these member functions?
constexpr const T& getData() const& { return data; }
constexpr T&& getData() && { return static_cast<T&&>(data); }
constexpr const T&& getData() const&& { return static_cast<const T&&>(data); }
template <class U>
requires std::constructible_from<T, U&&>
constexpr Proxy(U&& u) : data{std::forward<U>(u)} {}
// This constructor covers conversion from cvref of Proxy<U>, including non-const/const versions of copy/move constructor
template <class Other>
requires(IsProxy<std::decay_t<Other>> && std::constructible_from<T, decltype(std::declval<Other>().getData())>)
constexpr Proxy(Other&& other) : data{std::forward<Other>(other).getData()} {}
template <class Other>
requires(IsProxy<std::decay_t<Other>> && std::assignable_from<T&, decltype(std::declval<Other>().getData())>)
constexpr Proxy& operator=(Other&& other) {
var-constUnsubmitted
Done
ReplyInline Actions

This is to make ProxyIterator, not Proxy, indirectly writable, right? If so, can you please make it clear in the comment?

var-const: This is to make `ProxyIterator`, not `Proxy`, indirectly writable, right? If so, can you please…
data = std::forward<Other>(other).getData();
return *this;
}
// const assignment required to make ProxyIterator model std::indirectly_writable
template <class Other>
requires(IsProxy<std::decay_t<Other>> && std::assignable_from<const T&, decltype(std::declval<Other>().getData())>)
constexpr const Proxy& operator=(Other&& other) const {
data = std::forward<Other>(other).getData();
return *this;
}
// no specialised swap function that takes const Proxy& and no specialised const member swap
// Calling swap(Proxy<T>{}, Proxy<T>{}) would fail (pass prvalues)
// Compare operators are defined for the convenience of the tests
friend constexpr bool operator==(const Proxy&, const Proxy&)
requires std::equality_comparable<T>
= default;
var-constUnsubmitted
Done
ReplyInline Actions

These properties (std::indireclty_readable, std::indirectly_writable, anything else that's crucial for these classes to function correctly) should be static_asserted. You can add static assertions right after the class definitions.

var-const: These properties (`std::indireclty_readable`, `std::indirectly_writable`, anything else that's…
friend constexpr auto operator<=>(const Proxy&, const Proxy&)
requires std::three_way_comparable<T>
= default;
};
// This is to make ProxyIterator model `std::indirectly_readable`
template <class T, class U, template <class> class TQual, template <class> class UQual>
requires requires { typename std::common_reference_t<TQual<T>, UQual<U>>; }
struct std::basic_common_reference<Proxy<T>, Proxy<U>, TQual, UQual> {
using type = Proxy<std::common_reference_t<TQual<T>, UQual<U>>>;
};
template <class T, class U>
requires requires { typename std::common_type_t<T, U>; }
struct std::common_type<Proxy<T>, Proxy<U>> {
var-constUnsubmitted
Done
ReplyInline Actions

Nit: s/dereference/dereferenced/.

var-const: Nit: `s/dereference/dereferenced/`.
using type = Proxy<std::common_type_t<T, U>>;
};
var-constUnsubmitted
Done
ReplyInline Actions

Nit: s/e.g If/E.g. if/.

var-const: Nit: `s/e.g If/E.g. if/`.
// ProxyIterator
// ======================================================================
// It wraps `Base` iterator and when dereferenced it returns a Proxy<ref>
// It simulates C++23's zip_view::iterator but simplified to just wrap
// one base iterator.
// Note it forwards value_type, iter_move, iter_swap. e.g if the base
// iterator is int*,
// operator* -> Proxy<int&>
// iter_value_t -> Proxy<int>
// iter_move -> Proxy<int&&>
template <class Base>
var-constUnsubmitted
Done
ReplyInline Actions

Add line break to place struct on a different line.

var-const: Add line break to place `struct` on a different line.
struct ProxyIteratorBase {};
template <class Base>
requires std::derived_from<
typename std::iterator_traits<Base>::iterator_category,
std::input_iterator_tag>
struct ProxyIteratorBase<Base> {
using iterator_category = std::input_iterator_tag;
};
template <std::input_iterator Base>
consteval auto get_iterator_concept() {
if constexpr (std::random_access_iterator<Base>) {
return std::random_access_iterator_tag{};
} else if constexpr (std::bidirectional_iterator<Base>) {
return std::bidirectional_iterator_tag{};
} else if constexpr (std::forward_iterator<Base>) {
return std::forward_iterator_tag{};
} else {
return std::input_iterator_tag{};
}
}
template <std::input_iterator Base>
struct ProxyIterator : ProxyIteratorBase<Base> {
Base base_;
using iterator_concept = decltype(get_iterator_concept<Base>());
using value_type = Proxy<std::iter_value_t<Base>>;
using difference_type = std::iter_difference_t<Base>;
var-constUnsubmitted
Done
ReplyInline Actions

Nit: move requires to a different line?

var-const: Nit: move `requires` to a different line?
ProxyIterator()
requires std::default_initializable<Base>
= default;
constexpr ProxyIterator(Base base) : base_{std::move(base)} {}
template <class T>
requires std::constructible_from<Base, T&&>
constexpr ProxyIterator(T&& t) : base_{std::forward<T>(t)} {}
friend constexpr decltype(auto) base(const ProxyIterator& p) { return base(p.base_); }
// Specialization of iter_move
// If operator* returns Proxy<Foo&>, iter_move will return Proxy<Foo&&>
// Note std::move(*it) returns Proxy<Foo&>&&, which is not what we want as
// it will likely result in a copy rather than a move
friend constexpr Proxy<std::iter_rvalue_reference_t<Base>> iter_move(const ProxyIterator& p) noexcept {
return {std::ranges::iter_move(p.base_)};
}
// Specialization of iter_swap
// Note std::swap(*x, *y) would fail to compile as operator* returns prvalues
// and std::swap takes non-const lvalue references
friend constexpr void iter_swap(const ProxyIterator& x, const ProxyIterator& y) noexcept {
std::ranges::iter_swap(x.base_, y.base_);
}
// to satisfy input_iterator
constexpr Proxy<std::iter_reference_t<Base>> operator*() const { return {*base_}; }
constexpr ProxyIterator& operator++() {
++base_;
return *this;
}
constexpr void operator++(int) { ++*this; }
friend constexpr bool operator==(const ProxyIterator& x, const ProxyIterator& y)
requires std::equality_comparable<Base> {
return x.base_ == y.base_;
}
// to satisfy forward_iterator
constexpr ProxyIterator operator++(int)
requires std::forward_iterator<Base> {
auto tmp = *this;
++*this;
return tmp;
}
// to satisfy bidirectional_iterator
constexpr ProxyIterator& operator--()
requires std::bidirectional_iterator<Base> {
--base_;
return *this;
}
constexpr ProxyIterator operator--(int)
requires std::bidirectional_iterator<Base> {
auto tmp = *this;
--*this;
return tmp;
}
// to satisfy random_access_iterator
constexpr ProxyIterator& operator+=(difference_type n)
requires std::random_access_iterator<Base> {
base_ += n;
return *this;
}
constexpr ProxyIterator& operator-=(difference_type n)
requires std::random_access_iterator<Base> {
base_ -= n;
return *this;
}
constexpr Proxy<std::iter_reference_t<Base>> operator[](difference_type n) const
requires std::random_access_iterator<Base> {
return {base_[n]};
}
friend constexpr bool operator<(const ProxyIterator& x, const ProxyIterator& y)
requires std::random_access_iterator<Base> {
return x.base_ < y.base_;
}
friend constexpr bool operator>(const ProxyIterator& x, const ProxyIterator& y)
requires std::random_access_iterator<Base> {
return x.base_ > y.base_;
}
friend constexpr bool operator<=(const ProxyIterator& x, const ProxyIterator& y)
requires std::random_access_iterator<Base> {
return x.base_ <= y.base_;
}
friend constexpr bool operator>=(const ProxyIterator& x, const ProxyIterator& y)
requires std::random_access_iterator<Base> {
return x.base_ >= y.base_;
}
friend constexpr auto operator<=>(const ProxyIterator& x, const ProxyIterator& y)
requires(std::random_access_iterator<Base> && std::three_way_comparable<Base>) {
return x.base_ <=> y.base_;
}
friend constexpr ProxyIterator operator+(const ProxyIterator& x, difference_type n)
requires std::random_access_iterator<Base> {
return ProxyIterator{x.base_ + n};
}
friend constexpr ProxyIterator operator+(difference_type n, const ProxyIterator& x)
requires std::random_access_iterator<Base> {
return ProxyIterator{n + x.base_};
}
friend constexpr ProxyIterator operator-(const ProxyIterator& x, difference_type n)
requires std::random_access_iterator<Base> {
return ProxyIterator{x.base_ - n};
}
friend constexpr difference_type operator-(const ProxyIterator& x, const ProxyIterator& y)
requires std::random_access_iterator<Base> {
return x.base_ - y.base_;
}
};
static_assert(std::indirectly_readable<ProxyIterator<int*>>);
static_assert(std::indirectly_writable<ProxyIterator<int*>, Proxy<int>>);
template <class BaseSent>
struct ProxySentinel {
BaseSent base_;
ProxySentinel() = default;
constexpr ProxySentinel(BaseSent base) : base_{std::move(base)} {}
template <class Base>
requires std::equality_comparable_with<Base, BaseSent>
friend constexpr bool operator==(const ProxyIterator<Base>& p, const ProxySentinel& sent) {
return p.base_ == sent.base_;
}
};
#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
template <std::ranges::input_range Base>
requires std::ranges::view<Base>
struct ProxyRange {
Base base_;
constexpr auto begin() { return ProxyIterator{std::ranges::begin(base_)}; }
constexpr auto end() { return ProxySentinel{std::ranges::end(base_)}; }
constexpr auto begin() const
requires std::ranges::input_range<const Base> {
return ProxyIterator{std::ranges::begin(base_)};
}
constexpr auto end() const
requires std::ranges::input_range<const Base> {
var-constUnsubmitted
Done
ReplyInline Actions

Ultranit: opening brace normally goes to the preceding line.

var-const: Ultranit: opening brace normally goes to the preceding line.
return ProxySentinel{std::ranges::end(base_)};
}
};
template <std::ranges::input_range R>
requires std::ranges::viewable_range<R&&>
ProxyRange(R&&) -> ProxyRange<std::views::all_t<R&&>>;
#endif // !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
#endif // TEST_STD_VER > 17 #endif // TEST_STD_VER > 17
#endif // SUPPORT_TEST_ITERATORS_H #endif // SUPPORT_TEST_ITERATORS_H