[libcxx-commits] [PATCH] D105816: [libc++] Implement views::all_t and ranges::viewable_range

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/ranges/range.req/range.refinements/viewable_range.compile.pass.cpp:108
+static_assert(std::ranges::range<T7&>);
+static_assert(!std::ranges::view<T7>);
+static_assert(!std::constructible_from<T7, T7&>);
----------------
ldionne wrote:
> Quuxplusone wrote:
> > `s/T7/T7&/` here
> Hmm, are you sure about that? In all those places, I'm checking whether `ranges::view<remove_cvref_t<T>>` is satisfied. Here, `remove_cvref_t<T>` is `T7`, not `T7&`.
Ah, okay, I see what you're doing now. This is subtler than I'd like. I started to suggest that you might like to rewrite all 8 test cases in the form
```
{
  struct Tn { ... };
  using X = Tn;  // or Tn& as appropriate
  static_assert([! ]std::ranges::range<X>);
  static_assert([! ]std::ranges::view<std::remove_cvref_t<X>>);
  static_assert([! ]std::constructible_from<std::remove_cvref_t<X>, X>>);
  static_assert([! ]std::borrowed_range<X>>);
  static_assert([! ]std::viewable_range<X>>);
}
```
However, this is troublesome for the types like T2 and T2 that need to specialize `std::enable_borrowed_range`; you can't do that with a function-local struct type. So I have no great suggestion here.

Btw, does at least one of these tests fail when you go into `<__ranges/concepts.h>` and change each instance of `remove_cvref_t` to `remove_reference_t`? How about `decay_t`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105816