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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision.
philnik added a subscriber: jloser.
philnik added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/cpo.pass.cpp:66
 
+#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
     static_assert(std::is_same_v<std::ranges::range_reference_t<decltype(v2)>, std::tuple<std::pair<int&, int&>>>);
----------------
CaseyCarter wrote:
> philnik wrote:
> > Maybe guard this on `defined(__cpp_lib_tuple_like) && __cpp_lib_tuple_like >= 202207L`?
> As Hui commented below, the intent isn't to keep both alternatives. My intent is that these tests break loudly upon implementing P2165R4 so someone will open them up and remove the "old" code instead of leaving it dead. We could equivalently have _only_ the P2165R4 behavior and XFAIL these tests for libc++ but I prefer to validate the "wrong" behavior for the time being to defend against regressions that would cause the test to fail but in a different way.
> 
> Full disclosure: MSVCSTL has been implementing P2165 piecemeal, so we don't define the feature-test macro yet. Guarding with the macro wouldn't accomplish my goal of getting all of these tests up on MSVC.
Makes sense.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.zip/types.h:305-325
 template <class Base = int*>
 struct common_input_iterator {
   Base it_;
 
   using value_type = int;
   using difference_type = std::intptr_t;
   using iterator_concept = std::input_iterator_tag;
----------------
CaseyCarter wrote:
> philnik wrote:
> > Maybe we should lift this into `test_iterators.h`. This seems like the kind of thing that gets duplicated very easily.
> I'd prefer to land this as-is and leave any such code motion to a followup PR.
That's all right. @jloser already uploaded D141238.


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

https://reviews.llvm.org/D141216