[libcxx-commits] [PATCH] D108829: [libc++] Various cleanups in the ranges tests

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

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

Just skimmed, but it LGTM.

Might you be persuaded to do another cleanup patch replacing all the instances of `stdr::` with `std::ranges::`? Two days ago I was grepping for the `ranges::iterator_t` tests, and was mildly irked to find we "didn't have any" (because we had `stdr::iterator_t` instead).



================
Comment at: libcxx/test/std/ranges/range.access/range.access.begin/incomplete.verify.cpp:15
+// Test the libc++ specific behavior that we provide a better diagnostic when calling
+// std::ranges::begin on an array of incomplete types.
 
----------------
Grammar nit (throughout): "types" should be "type", since it's an array of (one single) incomplete type.


================
Comment at: libcxx/test/std/ranges/range.access/range.access.begin/incomplete.verify.cpp:23-25
 template <class T>
 requires(!std::invocable<begin_t&, T>)
 void f() {}
----------------
It might be more to the point to make this an actual SFINAE situation, like
```
template<class T> void f() requires std::invocable<begin_t&, T> {}
template<class T> void f() {}
```
https://godbolt.org/z/deP98eMvd

Also, calling this 48-line spew a "better" diagnostic is... generous. ;)


================
Comment at: libcxx/test/std/ranges/range.access/range.access.begin/incomplete.verify.cpp:30
   f<incomplete(&)[]>();
   // expected-error@*:* {{"`std::ranges::begin` is SFINAE-unfriendly on arrays of an incomplete type."}}
   // expected-error at -2 {{no matching function for call to 'f'}}
----------------
Tangent: Maybe the message shouldn't end with a period `.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108829