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

Fri Apr 16 15:02:12 PDT 2021

zoecarver added inline comments.

================
Comment at: libcxx/include/functional:3244
+  [[nodiscard]] constexpr bool operator()(_Tp &&__t, _Up &&__u) const {
+    return !(_VSTD::forward<_Tp>(__t) == _VSTD::forward<_Up>(__u));
+  }
----------------
Quuxplusone wrote:
> @ldionne, apparently we're free to use either expression `x != y` or `!(x == y)` here (because the whole point of `concept equality_comparable_with` is that those two expressions are interchangeable). I have a moderate preference for `x != y`, for simplicity and consistency with the ordinary `std::not_equal_to` — "we mean not-equal so let's write not-equal." What's your take? (See also my next comment below.)
> 
> https://en.cppreference.com/w/cpp/utility/functional/ranges/not_equal_to
I don't really care which thing we do. As for this comment and the one below, I'd be interested to hear other's opinions as well. I'm happy to implement whatever the consensus is. 

================
Comment at: libcxx/test/std/utilities/function.objects/range.cmp/equal_to.pass.cpp:36-37
+                                   NotEqualityComparable>);
+static_assert(std::is_invocable_v<std::ranges::equal_to, explicit_operators,
+                                  explicit_operators>);
+
----------------
Quuxplusone wrote:
> Drive-by FYI: the test class `explicit_operators` provides //seven// comparison operators (both the usual six //and// a redundant `operator<=>`). I'm not sure that was intentional.
I'm pretty sure it wasn't. I removed the spaceship operator. Not that it matters too much which are removed, but I think `explicit_operators` sort of indicates that the individual operators are declared. 

cc @cjdb 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100429