[libcxx-commits] [PATCH] D133220: fix errors on passing input iterator to `std::views::take`
Nikolas Klauser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Sep 9 08:42:59 PDT 2022
philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.
Could you add `[libc++]` to the commit message?
LGTM % nits. Thanks for the quick fix; sorry that I didn't review it earlier.
================
Comment at: libcxx/include/__ranges/take_view.h:229
template <class _Iter, class _Sent, subrange_kind _Kind>
+ requires requires{typename subrange<_Iter>;}
struct __passthrough_type<subrange<_Iter, _Sent, _Kind>> {
----------------
I think you said somewhere that GCC was OK without the requires clause. Could you check whether that's a GCC or Clang bug? (If you didn't, just ignore the comment)
================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take/adaptor.pass.cpp:130
// `views::take(string_view, n)` returns a `string_view`.
- {
- {
- std::string_view sv = "abcdef";
- std::same_as<decltype(sv)> decltype(auto) result = sv | std::views::take(3);
- assert(result.size() == 3);
- }
+ {{std::string_view sv = "abcdef";
+ std::same_as<decltype(sv)> decltype(auto) result = sv | std::views::take(3);
----------------
clang-format seems to be quite unhappy here. Could you file a bug and fix the formatting for now?
================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take/adaptor.pass.cpp:199
+ using Sent = sentinel_wrapper<Iter>;
+ std::ranges::subrange<Iter, Sent> r{Iter{input}, Sent{Iter{input + 3}}};
+ [[maybe_unused]] auto tv = std::views::take(std::move(r), 1);
----------------
Nit: Use CTAD?
================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take/adaptor.pass.cpp:200
+ std::ranges::subrange<Iter, Sent> r{Iter{input}, Sent{Iter{input + 3}}};
+ [[maybe_unused]] auto tv = std::views::take(std::move(r), 1);
+ auto it = tv.begin();
----------------
The `[[maybe_unused]]` seems to be unnecessary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133220/new/
https://reviews.llvm.org/D133220
More information about the libcxx-commits
mailing list