[libcxx-commits] [PATCH] D133317: implement `std::views::istream`

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__ranges/istream_view.h:104
+struct __fn {
+  template <class _Up, class _UnCVRef = remove_cvref_t<_Up>>
+    requires derived_from<_UnCVRef, basic_istream<typename _UnCVRef::char_type,
----------------
philnik wrote:
> I'm not sure the extra indirection is worth it. Why do you need to strip cv-qualifiers at all?
Because usually `_Up` is deduced to `std::istringstream&`, and a reference type doesn't have nested member typedef `typename _Up::char_type`
Note I have to strip cv for this reason in lots of places within this function


================
Comment at: libcxx/test/std/ranges/range.factories/range.istream.view/range.concept.compile.pass.cpp:15
+
+template <class Val, class CharT, class Traits = std::char_traits<CharT>>
+concept HasIstreamView = requires { typename std::ranges::basic_istream_view<Val, CharT, Traits>; };
----------------
philnik wrote:
> Why don't you test `Traits` in any way?
The standard doesn't have any constraint on `Traits` other than being used in `std::basic_istream<CharT, Traits>`. However, `basic_istream` is a classic template which isn't constrained. passing an invalid `Traits` to `std::basic_istream` would just hard error. so
`static_assert(!HasIstreamView<int, char, NotTraits>)` would just hard error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133317