[libcxx-commits] [PATCH] D100269: [libcxx] adds concept `range` and range aliases

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/CMakeLists.txt:157
   ranges
-  regex
   scoped_allocator
----------------
Mordante wrote:
> This seems unintentional.
Oops.


================
Comment at: libcxx/include/__ranges/concepts.h:43
+using range_difference_t = iter_difference_t<iterator_t<R> >;
+
+template <range R>
----------------
Mordante wrote:
> Shouldn't
> ```
> template<sized_­range R>
>     using range_size_t = decltype(ranges::size(declval<R&>()));
> ```
> be included here? Or is it somewhere else? If it's somewhere else can you add a comment like you did for `iterator_t`?
Please see the commit message.


================
Comment at: libcxx/include/ranges:27-30
+//   template<class T>
+//     using iterator_t = decltype(ranges::begin(declval<T&>()));
+//   template<class T>
 //     using iterator_t = decltype(ranges::begin(declval<T&>()));
----------------
miscco wrote:
> Is there some duplication here?
Where?


================
Comment at: libcxx/test/std/strings/basic.string/range_concept_conformance.compile.pass.cpp:22
+static_assert(std::ranges::range<std::string>);
+static_assert(std::ranges::range<std::string const>);
----------------
Mordante wrote:
> Can you also test `std::basic_string` with other character types, `wchar_t`, `char8_t`, `char16_t`, and `char32_t`?
I'm not convinced those will yield anything useful since they're aliases, not specialisations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100269