[libcxx-commits] [PATCH] D116950: [libc++] Add the std::ranges::drop range adaptor object

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__ranges/drop_view.h:142-143
+
+    template<class _Tp>
+    constexpr bool __is_passthrough_specialization<span<_Tp, dynamic_extent>> = true;
+
----------------
https://eel.is/c++draft/range.drop#overview-2.2.1
IIUC, this should also permit fixed spans like `std::span<T, 42>`. This needs a regression test.


================
Comment at: libcxx/include/__ranges/drop_view.h:151-152
+
+    template<class _Iter, class _Sent, subrange_kind _Kind>
+    constexpr bool __is_passthrough_specialization<subrange<_Iter, _Sent, _Kind>> = true;
+
----------------
This smells like it doesn't exactly match what's supposed to happen in
https://eel.is/c++draft/range.drop#overview-2.2.4
Note that `subrange`s with //StoreSize//=true are handled in https://eel.is/c++draft/range.drop#overview-2.3 instead. It's a whole maze.

@var-const, you should definitely take a look at my D115122 for comparison. On glancing at it right now, I'm not sure that I do the `subrange` bullet points correctly, either, though!




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116950