Details
- Reviewers
cjdb ldionne EricWF • Quuxplusone - Group Reviewers
Restricted Project - Commits
- rGbede7523b1b4: [libcxx][ranges] Add `contiguous_iterator`.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/contiguous_iterator.compile.pass.cpp | ||
---|---|---|
28–32 | Most importantly, please check the test iterators from test_iterators.h — e.g. static_assert(std::contiguous_iterator<contiguous_iterator<int*>>); Any test iterators that don't match the concepts will want to be fixed. |
libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/contiguous_iterator.compile.pass.cpp | ||
---|---|---|
28–32 | I have those a few lines above: static_assert(!std::contiguous_iterator<input_iterator<int*> >); static_assert(!std::contiguous_iterator<forward_iterator<int*> >); static_assert(!std::contiguous_iterator<bidirectional_iterator<int*> >); static_assert(!std::contiguous_iterator<random_access_iterator<int*> >); static_assert(std::contiguous_iterator<contiguous_iterator<int*> >); |
Missing conformance tests:
- deque (negative test)
- vector
- span
- string
- reverse_iterator
LGTM, pending CI and added conformance tests.
libcxx/include/__memory/pointer_traits.h | ||
---|---|---|
201 | I'm going to make this into a separate PR with test. |
Fix the two failing tests.
libcxx/test/std/containers/sequences/vector.bool/iterator_concept_conformance.compile.pass.cpp | ||
---|---|---|
26 | Just want to confirm this is correct: vector<bool> is not a contiguous container (while normal vector is). |
libcxx/test/std/containers/sequences/vector.bool/iterator_concept_conformance.compile.pass.cpp | ||
---|---|---|
26 | Correct. |
LGTM % comments
libcxx/include/__iterator/concepts.h | ||
---|---|---|
16 | At some point: Alphabetize. | |
libcxx/test/std/containers/sequences/vector.bool/iterator_concept_conformance.compile.pass.cpp | ||
26 | Please keep static_assert(std::random_access_iterator<reverse_iterator>); (just add the new negative asserts). | |
libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/contiguous_iterator.compile.pass.cpp | ||
15 | concept contiguous_iterator; | |
46–50 | (A) this could be operator<=>? | |
libcxx/test/std/iterators/predef.iterators/reverse.iterators/iterator_concept_conformance.compile.pass.cpp | ||
42 | Please keep the assert for static_assert(std::random_access_iterator<reverse_contiguous_iterator>); (just add the new negative assert). This also applies anywhere else in this PR that you removed a positive assert without replacing it with a stronger positive assert (if any such places exist). |
Remove extra requires that's no longer needed now that to_address is SFINAE friendly.
At some point: Alphabetize.