[libcxx-commits] [PATCH] D122806: [libc++] add zip_view and views::zip for C++23

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/ctor.views.pass.cpp:20
+
+constexpr bool test() {
+
----------------
philnik wrote:
> philnik wrote:
> > You should also check here that
> > - the arguments are always moved
> > - the arguments are moved exactly once
> > - all the different range types (`input_range`, `forward_range`, `output_range` etc.); See the any of the ranges algorithm tests for an example of that.
> > - the constructor is `explicit`
> It looks like you aren't checking the that `zip` accepts all the ranges types.
added more tests for bidi, random_access, contiguous. 
output only ranges are not supported by the zip_view class. There was already a test in range.concept.compile.pass.cpp to check that output_range isn't supported.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/iterator/iter_move.pass.cpp:28
+
+constexpr bool test() {
+  {
----------------
philnik wrote:
> philnik wrote:
> > You should check here that the `iter_move` of the iterator is called properly.
> I think you aren't checking that a custom `iter_move` is called properly. This should be quite similar to the `iter_swap` test.
Could you please elaborate? The `iter_move` test looks really similar to `iter_swap` test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122806