[libcxx-commits] [PATCH] D104262: [libcxx][ranges] Add contiguous_range.
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jun 14 13:23:34 PDT 2021
Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.
LGTM % comments.
================
Comment at: libcxx/test/std/containers/sequences/deque/range_concept_conformance.compile.pass.cpp:26
static_assert(stdr::random_access_range<range>);
+static_assert(stdr::contiguous_range<range>);
static_assert(!stdr::view<range>);
----------------
`deque<T>` is not a contiguous range. Add `!` here and below.
================
Comment at: libcxx/test/std/containers/sequences/vector.bool/range_concept_conformance.compile.pass.cpp:26
static_assert(stdr::random_access_range<range>);
+static_assert(stdr::contiguous_range<range>);
static_assert(!stdr::view<range>);
----------------
`vector<bool>` is not a contiguous range. Add `!` here and below.
================
Comment at: libcxx/test/std/ranges/range.req/range.refinements/contiguous_range.compile.pass.cpp:14
+// template<range _Rp>
+// concept random_access_range;
+
----------------
`contiguous_range`
================
Comment at: libcxx/test/std/ranges/range.req/range.refinements/contiguous_range.compile.pass.cpp:42-46
+struct BadDataMember {
+ int *begin();
+ int *end();
+ char *data();
+};
----------------
I was initially confused by the name, as this class has //no// data members... but then I saw: the trick is that `X::data()` member function has a return type that is different from `std::add_pointer_t<ranges::range_reference_t<X>>`.
I think it'd be worth replacing this test (or at augmenting it) with both of these:
```
struct WrongConstness {
const int *begin() const;
const int *end() const;
int *data() const;
};
static_assert(std::ranges::random_access_range<WrongConstness>);
static_assert(!std::ranges::contiguous_range<WrongConstness>);
struct NotPtr {
contiguous_iterator<const int*> begin() const;
contiguous_iterator<const int*> end() const;
contiguous_iterator<const int*> data() const;
};
static_assert(std::ranges::random_access_range<NotPtr>);
static_assert(!std::ranges::contiguous_range<NotPtr>);
```
================
Comment at: libcxx/test/std/ranges/range.req/range.refinements/subsumption.compile.pass.cpp:28
template<std::ranges::range R>
-requires std::input_iterator<std::ranges::iterator_t<R> >
+requires std::input_iterator<std::ranges::iterator_t<R>>
[[nodiscard]] constexpr bool check_input_range_subsumption() {
----------------
Whitespace: Please indent `requires` by one level (here and throughout). E.g.
```
template<...>
requires ...
bool foo() {
return ...
}
```
It'd be nice to remove the [[nodiscard]]s while you're at it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104262/new/
https://reviews.llvm.org/D104262
More information about the libcxx-commits
mailing list