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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for the patch!

I am not sure I follow the bit about the implementation not being as efficient as could be. Are you essentially suggesting that the 3rd implementation in https://godbolt.org/z/6sTfePhPq would be more efficient but more complicated?



================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way_comp.pass.cpp:40
+  // Identical arrays
+  assert(testLexicographicalCompare(array{90, 81}, array{10, 11}, std::strong_ordering::equal));
+  // "Greater" on 2nd element
----------------
We should also be testing with iterator archetypes like you've done for the non-`comp` test. Is there a reason why it doesn't apply here?


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way_comp.pass.cpp:42
+  // "Greater" on 2nd element
+  assert(testLexicographicalCompare(array{70, 81}, array{10, 12}, std::strong_ordering::less));
+  // "Less" on 2nd element
----------------
We should also be testing with more than just `strong_ordering`. The same applies to the non-`comp` test IIUC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131395