[libcxx-commits] [PATCH] D117940: [libc++] [ranges] Implement std::ranges::distance

[Nikolas Klauser via Phabricator via libcxx-commits]

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

LGTM % nits



================
Comment at: libcxx/docs/Status/RangesPaper.csv:74
 | `ranges::next <https://llvm.org/D102563>`_
-| `ranges::prev <https://llvm.org/D102564>`_",[iterator.concepts],Christopher Di Bella,In progress
+| `ranges::prev <https://llvm.org/D102564>`_",[iterator.concepts],Christopher Di Bella,Complete
 [predef.iterators],Updates to predefined iterators.,"| [iterator.concepts]
----------------



================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.distance/iterator_sentinel.pass.cpp:33
+    assert(std::ranges::distance(It(a), last) == 0);
+    assert(std::ranges::distance(It(a), std::move(last)) == 0);
+  }
----------------
Since you're testing 3/4 possible combinations also test the fourth? ditto below.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.distance/iterator_sentinel.pass.cpp:112-113
+    Sent last = Sent(It(a + 3));
+    static_assert(!std::is_invocable_v<decltype(std::ranges::distance), It&, Sent&>);
+    static_assert(!std::is_invocable_v<decltype(std::ranges::distance), It&, Sent&&>);
+    assert(std::ranges::distance(It(a), last) == 3);
----------------
Please move the `static_assert`s together.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.distance/range.pass.cpp:52
+    assert(std::ranges::distance(r) == 3);
+    assert(std::ranges::distance(static_cast<R&&>(r)) == 3);
+    static_assert(!std::is_invocable_v<decltype(std::ranges::distance), const R&>);
----------------
Why not `std::move`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117940