[libcxx-commits] [PATCH] D131395: [libc++] Implement `lexicographical_compare_three_way`

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__algorithm/lexicographical_compare_three_way.h:24
+
+template <class InputIterator1, class InputIterator2, class Cmp>
+_LIBCPP_HIDE_FROM_ABI constexpr auto lexicographical_compare_three_way(
----------------
huixie90 wrote:
> 
Thanks for updating these names but I think the last one `Cmp` has not been unglified


================
Comment at: libcxx/include/__algorithm/lexicographical_compare_three_way.h:54
+    _InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2, _InputIterator2 __last2) {
+  return lexicographical_compare_three_way(__first1, __last1, __first2, __last2, compare_three_way());
+}
----------------
I believe the spec does not want us to trigger ADL


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:41-43
+  auto result =
+      std::lexicographical_compare_three_way(Iter1{a.begin()}, Iter1{a.end()}, Iter2{b.begin()}, Iter2{b.end()});
+  ASSERT_SAME_TYPE(decltype(result), Order);
----------------
optional: In other C++20 algorithm tests, we usually do
```
std::same_as<ExpectedType> result = std::foo_bar_algorithm(...);
```
But feel free to ignore this.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:44
+  ASSERT_SAME_TYPE(decltype(result), Order);
+  return expected == result;
+}
----------------
extra nit: In other tests, we usually just `assert(expected == result)` here to save writing all the `assert`s on the caller site.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131395