[libcxx-commits] [PATCH] D103272: [libc++] Refactor the ranges::prev and ranges::next tests

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver accepted this revision as: zoecarver.
zoecarver added a comment.

LGTM, thanks for the cleanup.



================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/constraints.verify.cpp:19-25
 void proper_constraints() {
   auto p = std::unique_ptr<int>();
   std::ranges::next(p);    // expected-error {{no matching function for call}}
   std::ranges::next(p, p); // expected-error {{no matching function for call}}
   std::ranges::next(p, 5); // expected-error {{no matching function for call}}
   std::ranges::next(p, 7); // expected-error {{no matching function for call}}
 }
----------------
Quuxplusone wrote:
> It would be even nicer to make this a real SFINAE test and enable it for gcc-10/gcc-11.
I agree. I think wherever possible we should move away from `.verify` tests. This could just be `is_invocable_v`. 

The only place we should really use the `.verify` tests is when we're checking a static_assert. 


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/iterator_sentinel.pass.cpp:81
   check_assignable_case<contiguous_iterator<range_t::const_iterator> >(6);
-  check_assignable_case<output_iterator<range_t::iterator> >(7);
 
----------------
Do we want to add other checks for output_iterator? Why did you remove this check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103272