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

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/algorithm:5633
+    for (; __first1 != __last1 && __first2 != __last2; void(++__first1), void(++__first2))
+    if (auto __cmp = __comp(*__first1, *__first2); __cmp != 0)
+        return __cmp;
----------------
miscco wrote:
> The identation here is off. Could you please add braces so that it is clear where the loop ends
clang-format doesn't seem to care about this file, so I've had to do it manually. Weird.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp:40
+    int ib[] = {1, 2, 3};
+    assert(std::lexicographical_compare_three_way(Iter1(ia),   Iter1(ia+sa), Iter2(ib),   Iter2(ib+2)) == std::strong_ordering::greater);
+    assert(std::lexicographical_compare_three_way(Iter1(ib),   Iter1(ib+2),  Iter2(ia),   Iter2(ia+sa)) == std::strong_ordering::less);
----------------
miscco wrote:
> These are rather subtle tests. To make it imediately clear what you are actually testing I would appreciate if you could go the extra mile and add comments
> 
> // lexicographical_compare_three_way(a, b),  range b shorter than range a
> 
> // lexicographical_compare_three_way(a, b),  range a shorter than range b
Done, but you might ask for further improvements.


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

https://reviews.llvm.org/D80902