[libcxx-commits] [PATCH] D100255: [libcxx] adds `range` access CPOs

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/test/std/ranges/range.access/array_access.h:30
+  static_assert(!std::invocable<decltype(std::ranges::cpo), double[321]>);                                             \
+  static_assert(!std::invocable<decltype(std::ranges::cpo), long(&)[]>);                                               \
+                                                                                                                       \
----------------
zoecarver wrote:
> How about rvalue array types?
Line 29 takes care of this 🙂 


================
Comment at: libcxx/test/std/ranges/range.range/iterator_t.compile.pass.cpp:24
+
+namespace stdr = std::ranges;
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > Bikeshed: this seems much too easy to typo. I've been using `namespace rg = std::ranges` in my own slide code; anyone got thoughts?
> > Of course, I'm also not a huge fan of this test, since it has dependencies on so many containers. It would be better to do just like
> > ```
> > #include <ranges>
> > #include <type_traits>
> > #include "test_iterators.h"
> > struct X {
> >     forward_iterator<int> begin();
> >     forward_iterator<int> end();
> >     forward_iterator<char> begin() const;
> >     forward_iterator<char> end() const;
> > };
> > static_assert(std::is_same_v<std::ranges::iterator_t<X>, forward_iterator<int>>);
> > static_assert(std::is_same_v<std::ranges::iterator_t<const X>, forward_iterator<char>>);
> > static_assert(std::is_same_v<std::ranges::iterator_t<double[10]>, double*>);
> > ```
> > Bikeshed: this seems much too easy to typo. I've been using namespace `rg = std::ranges` in my own slide code; anyone got thoughts?
> 
> 🤷 I'm happy to bikeshed on Discord (not in-review please).
> 
> > Of course, I'm also not a huge fan of this test, since it has dependencies on so many containers. It would be better to do just like
> 
> Part of the point of this test is to ensure backwards compatibility with all of our containers. That was a design goal for ranges, and should be reflected here IMO.
Since no discussion occurred, I replaced `stdr::` in places where it could be ambiguous with `std::ranges`. Places where it isn't ambiguous can remain as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100255