[libcxx-commits] [PATCH] D106708: [libc++] Add range_size_t

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM mod comments.



================
Comment at: libcxx/include/__ranges/concepts.h:73
+  template<sized_range _Rp>
+  using range_size_t = decltype(ranges::size(declval<_Rp&>()));
+
----------------
This is actually from section [range.range], not [range.sized]. I doubt we care.


================
Comment at: libcxx/include/ranges:38-39
 
-  template<class T>
-    using iterator_t = decltype(ranges::begin(declval<T&>()));
-  template<class T>
-    using iterator_t = decltype(ranges::begin(declval<T&>()));
+  template<class R>
+    using iterator_t = decltype(ranges::begin(declval<R&>()));
   template<range R>
----------------
No, `T` was correct. https://eel.is/c++draft/ranges#syn
It's noteworthy that `iterator_t` is not constrained — `class T` is correct, not `range T`. I don't know why.


================
Comment at: libcxx/test/std/ranges/range.req/range.range/range_size_t.compile.pass.cpp:28
+
+static_assert(std::same_as<std::ranges::range_size_t<Range>, short>);
----------------
Please add SFINAE-friendliness tests. I recommend replacing this whole file with:
```
template<class T>
concept has_size_t = requires { typename std::ranges::range_size_t<T>; };

struct A { int *begin(); int *end(); short size(); };
static_assert(std::same_as<std::ranges::range_size_t<A>, short>);
static_assert(std::same_as<std::ranges::range_size_t<A&>, short>);
static_assert(std::same_as<std::ranges::range_size_t<A&&>, short>);
static_assert(!has_size_t<const A>);
static_assert(!has_size_t<const A&>);
static_assert(!has_size_t<const A&&>);

struct B { int *begin(); int *end(); };
static_assert(std::same_as<std::ranges::range_size_t<B>, std::size_t>);
static_assert(std::same_as<std::ranges::range_size_t<B&>, std::size_t>);
static_assert(std::same_as<std::ranges::range_size_t<B&&>, std::size_t>);
static_assert(!has_size_t<const B>);
static_assert(!has_size_t<const B&>);
static_assert(!has_size_t<const B&&>);

struct C { bidirectional_iterator<int*> begin(); bidirectional_iterator<int*> end(); };
static_assert(!has_size_t<C>);
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106708/new/

https://reviews.llvm.org/D106708