[libcxx-commits] [PATCH] D80902: [libcxx] adds lexicographical_compare_three_way

Marek Kurdej via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jun 3 04:54:01 PDT 2020

curdeius added inline comments.

Comment at: libcxx/include/compare:644
   constexpr _CCC _Cat = sizeof...(_Ts) == 0 ? _StrongOrd
-      : __compute_comp_type(__type_kinds);
+      : __compute_comp_type<sizeof...(_Ts)>(__type_kinds);
   if constexpr (_Cat == _None)
Nit: you could avoid passing the size explicitly if you passed the array by reference.

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:82
+  test_less<Iter1, Iter2>(a, b);
+  test_equal<Iter1, Iter2>(a, a);
That's already better, even if personally I would create a different array in each of the test_x functions.

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:89
+int main(int, char**) {
+  test<input_iterator<const int*>, input_iterator<const int*> >();
+  test<input_iterator<const int*>, forward_iterator<const int*> >();
You can move all those calls to `test<Iterator, Iterator>` to another function and just call it twice from main (once in static_assert).

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:24
+TEST_CONSTEXPR bool test_constexpr() {
+    int ia[] = {1, 2, 3};
cjdb wrote:
> curdeius wrote:
> > Constexpr test is... minimal to say the least. I think it would be better to have a single constexpr function (for applicable cases) and call it once on runtime and once on compile time.
> I can do that. This test is (mostly) a carbon copy of the test for std::lexicographical_compare, so many of your comments are also applicable to that test as well. That doesn't excuse me from improving this test, so I've applied the changes here.
> Does libc++ have a way to assert based on whether it's compile-time or run-time yet?
I'm not sure if I understand your question correctly. There is `(__libcpp_)is_constant_evaluated`, but there is, to my knowledge, nothing that asserts that a path is taken on runtime/compile time.

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way_comp.pass.cpp:12
+// template<InputIterator Iter1, InputIterator Iter2, CopyConstructible Compare>
+//   constexpr bool
+//   lexicographical_compare_three_way(Iter1 first1, Iter1 last1,
cjdb wrote:
> curdeius wrote:
> > Bool?
> Fixed. How many of your comments from the above are also applicable here?
Probably all.



More information about the libcxx-commits mailing list