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

[Tim Song via Phabricator via libcxx-commits]

tcanens 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));
+  }
----------------
cjdb wrote:
> tcanens wrote:
> > Quuxplusone wrote:
> > > tcanens wrote:
> > > > cjdb wrote:
> > > > > zoecarver wrote:
> > > > > > 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. 
> > > > > Per [[ http://eel.is/c++draft/range.cmp | range.cmp ]], `not_equal_to::operator()` has effects equivalent to `return !ranges::equal_to{}(std::forward<T>(t), std::forward<U>(u))`.
> > > > > 
> > > > > I'd actually prefer to see all of the implementations return the expressions in the standard wording, verbatim.
> > > > I've removed the claim on cppreference that this can be implemented with `!=`; the convertible-to-pointer corner case means that it can't - consider a case where `==` results in a built-in comparing pointers but `!=` doesn't; we attach no semantic requirements to the `!=` in such a case.
> > > > 
> > > > 
> > > @tcanens: Uh-oh. And the resolution of https://cplusplus.github.io/LWG/issue3530 doesn't affect your statement?
> > > Could you post an example test case where the difference between `!ranges::equal_to{}(FWD(t), FWD(u))` and `FWD(t) != FWD(u)` is observable?
> > > And I would bet that the difference between `!ranges::equal_to{}(FWD(t), FWD(u))` and `!(FWD(t) == FWD(u))` is also observable, because the latter might be applying `!` to a type other than `bool`?
> > ```
> > struct T {
> >     operator void*() const { return nullptr; }
> >     friend bool operator!=(T, T) { return true; }
> > };
> > ```
> > 
> > LWG3530 means that implementations don't have to use heroic efforts to detect and support types for which the other comparisons are ill-formed, but here the comparison is well-formed and "just" gives nonsensical results. It doesn't seem to be worth the extra specification effort to make this pathological case undefined, since the comparison function objects are the only ones affected, and they still produces meaningful results.
> > 
> > > And I would bet that the difference between `!ranges::equal_to{}(FWD(t), FWD(u))` and `!(FWD(t) == FWD(u))` is also observable, because the latter might be applying `!` to a type other than bool?
> > 
> > //`boolean-testable`// has enough semantic requirements for the latter to work (and for the carve-out case the result type is definitely `bool`). I would consider the difference unobservable.
> >  And the resolution of https://cplusplus.github.io/LWG/issue3530 doesn't affect your statement?
> 
> No, because that's exclusively to do with pointers.
> 
> > Could you post an example test case where the difference between `!ranges::equal_to{}(FWD(t), FWD(u))` and `FWD(t) != FWD(u)` is observable?
> 
> ```
> enum class X { a, b };
> 
> [[nodiscard]] bool operator!=(X const x, X const y) noexcept
> {
>   auto const result = static_cast<int>(x) != static_cast<int>(y);
>   std::cout << result << '\n';
>   return result;
> }
> ```
> 
> Same for `t > u` below.
> 
> > And I would bet that the difference between `!ranges::equal_to{}(FWD(t), FWD(u))` and `!(FWD(t) == FWD(u))` is also observable, because the latter might be applying `!` to a type other than `bool`?
> 
> Sadly. I think `!bool(FWD(t) == FWD(u))` is okay because `FWD(t) == FWD(u)` needs to be converted to `bool` in `ranges::equal_to{}(FWD(t), FWD(u))` anyway.
I think the enum example goes a bit too far. For types required to model `equality_comparable` (or even the weaker form), I'm pretty sure the intent is that implementations can use `x != y` and `!(x ==y)` mostly interchangeably.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100429