[libcxx-commits] [PATCH] D100429: [libc++][ranges] Add range.cmp: equal_to, not_equal_to, less, etc.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/test/std/utilities/function.objects/range.cmp/less.pass.cpp:60
+  // test total ordering of int* for less<int*> and less<void>.
+  do_pointer_comparison_test<std::less>();
+
----------------
BLOCKER: Did you mean `/std::less/std::ranges::less/`?  But then that won't compile, right? because `std::ranges::less` is not a class template anymore?

The good news is that literally everything else in this patch is good; I downloaded the raw diff and went through it in `git diff`, so it wasn't until the very end that I got to the newly added files and found the lack of tests for `std::ranges::less`. :)

I recommend splitting out the trivial whitespace diffs and just landing them for now.
(This will also help in case you end up having to revert the functional change later.)


================
Comment at: libcxx/test/support/compare_types.h:8-9
 //===----------------------------------------------------------------------===//
 #ifndef TEST_STD_CONCEPTS_COMPARISON_TYPES_H
 #define TEST_STD_CONCEPTS_COMPARISON_TYPES_H
 
----------------
Quuxplusone wrote:
> Probably a good idea to fix the include guard's name.
Throughout: `s/TEST_SUPPORT_COMPARISON_TYPES_H/TEST_SUPPORT_COMPARE_TYPES_H/`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100429