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

[Adrian Vogelsgesang via Phabricator via libcxx-commits]

avogelsgesang added inline comments.


================
Comment at: libcxx/include/module.modulemap.in:291
       module lexicographical_compare         { private header "__algorithm/lexicographical_compare.h" }
+      module lexicographical_compare_three_way { private header "__algorithm/lexicographical_compare_three_way.h" }
       module lower_bound                     { private header "__algorithm/lower_bound.h" }
----------------
Mordante wrote:
> Can you indent the other `{`s to the same level.
I just realized that there is already a pattern how to deal with overly long names, see `uniform_random_bit_generator_adaptor`. Did the same here now...


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:27
+
+using std::array;
+
----------------
Mordante wrote:
> Please use the qualified name in the tests.
any particular reason? Would it be fine if I used a function-local `using std::array`?


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way_comp.verify.cpp:18
+//       -> decltype(comp(*b1, *b2));
+
+#include <array>
----------------
Mordante wrote:
> Can you also test with iterators with invalid difference types?
do we already have such a test utility somewhere? I couldn't find anything useful in `test_iterators.h`


================
Comment at: libcxx/test/support/test_comparisons.h:253
+  friend std::partial_ordering operator<=>(PartialComp, PartialComp) = default;
+};
+#endif
----------------
Mordante wrote:
> Why not complete the set with `std::strong_ordering`?
because for `strong_ordering` you can simply use a plain `int`. Or should I still add it for completeness?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131395