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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked 9 inline comments as done.
ldionne added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/iterator.pass.cpp:22-28
+  int range[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
+  assert(&*std::ranges::next(It(&range[0])) == &range[1]);
+  assert(&*std::ranges::next(It(&range[1])) == &range[2]);
+  assert(&*std::ranges::next(It(&range[2])) == &range[3]);
+  assert(&*std::ranges::next(It(&range[3])) == &range[4]);
+  assert(&*std::ranges::next(It(&range[4])) == &range[5]);
+  assert(&*std::ranges::next(It(&range[5])) == &range[6]);
----------------
Quuxplusone wrote:
> Simply:
> ```
> int range[4] = {};
> assert(base(std::ranges::next(It(range))) == range+1);
> assert(base(std::ranges::next(It(range+3))) == range+4);
> ```
I wanted the test to be agnostic to the fact that `It` is an iterator archetype or not.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/iterator.pass.cpp:43-44
 int main(int, char**) {
-  static_assert(check_iterator());
-  check_iterator();
+  static_assert(test());
+  test();
   return 0;
----------------
Quuxplusone wrote:
> Bikeshed: Traditionally we put the runtime test first and the longer static_assert second.
Indeed, will fix throughout.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/iterator_sentinel.pass.cpp:52-53
+  {
+    auto result = std::ranges::next(std::move(it), std::move(last));
+    assert(&*result == expected);
+  }
----------------
Quuxplusone wrote:
> Here and throughout this function, the `auto`s are really confusing. Is this `auto` the same thing as `I`?
> Is `&*result` the same thing as `base(result)`? (`base` being an overload set defined in "test_iterators.h".)
As I said elsewhere, `&*` is to avoid relying on the fact that it's an iterator archetype. Will fix `auto`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103272