[libcxx-commits] [PATCH] D141216: [libc++][test] zip_view test cleanups

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jan 8 10:09:56 PST 2023


huixie90 accepted this revision.
huixie90 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/ctor.default.pass.cpp:53
     auto it = v.begin();
-    using Pair = std::pair<const int&, const int&>;
-    assert(*it++ == Pair(buff[0], buff[0]));
-    assert(*it++ == Pair(buff[1], buff[1]));
-    assert(*it == Pair(buff[2], buff[2]));
+#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
+    using Value = std::pair<const int&, const int&>;
----------------
In theory I guess we should check the feature test macro `__cpp_lib_tuple_like`, but I think I prefer your solution. We are going to implement the paper without supporting the previous behaviour. so after implementing the paper, a test failure would be preferable over silently selecting another branch.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/types.h:322
 
-  constexpr int& operator*() const { return *it_; }
+  constexpr auto& operator*() const { return *it_; }
 
----------------
Thanks for the fix. I think this function is not used in the tests. The main usage of this class is to test the `end` function returns the correct type and compares with the iterator correctly (without calling operator*)
btw, I prefer `decltype(auto)` here even though it does not really matter.


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

https://reviews.llvm.org/D141216



More information about the libcxx-commits mailing list