[libcxx-commits] [PATCH] D127130: [libc++] Implement ranges::lexicographical_compare

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const requested changes to this revision.
var-const added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/ranges_lexicographical_compare.h:63
+                  _Proj2 __proj2 = {}) const {
+    return __lexicographical_compare_impl(std::move(__first1), std::move(__last1),
+                                          std::move(__first2), std::move(__last2),
----------------
Nit: add the include.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp:132
+
+constexpr bool test() {
+  test_iterators2<cpp17_input_iterator<int*>, sentinel_wrapper<cpp17_input_iterator<int*>>>();
----------------
Please also add tests with a non-default projection or a non-default comparator.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.lex.comparison/ranges.lexicographical_compare.pass.cpp:189
+      int a[] = {1, 2, 3, 4, 5};
+      auto ret = std::ranges::lexicographical_compare(std::begin(a), std::end(a), std::begin(a), std::end(a), pred, proj1, proj2);
+      assert(!ret);
----------------
Question: why is `std::begin` necessary here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127130