[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 added inline comments.


================
Comment at: libcxx/test/std/utilities/function.objects/range.cmp/less.pass.cpp:23
+// TODO: find a type that has a builtin type that has operator< but is not totally ordered
+// to test LWG3530.
+
----------------
zoecarver wrote:
> Can anyone think of a type for this? If not, I'll just remove the comment. 
IIUC, https://cplusplus.github.io/LWG/issue3530 seems to be talking about the "`a < b` resolves to builtin pointer comparison" nonsense that spawned about half of https://quuxplusone.github.io/blog/2019/01/20/std-less-nightmare/ . No library actually implements that issue's //`BUILTIN-PTR-THREE-WAY`// because it's literally not implementable without compiler magic. But if you did want a test for LWG3530, just for completeness, just to prove that libc++ doesn't implement the unimplementable magic ;) then you would write something like this:
```
struct A {
    operator int*() const;
    friend bool operator>=(A, A) = delete;
};
```
Here `A a; a <=> a` is well-formed and resolves to built-in pointer comparison... and yet, `a >= a` is ill-formed. So LWG3530 is saying that actually `std::three_way_compare()(a, a)` should also be ill-formed.
https://godbolt.org/z/KxWEa7q3s
This is of course absolutely crazy and not at all in keeping with the "STL classic" philosophy of lifted operators like std::less, std::equal_to, etc.; the extra syntactic constraints seem to have leaked over from Ranges. But unless we're going to file a library defect //against// the resolution of LWG3530, I think we're stuck implementing it.


================
Comment at: libcxx/test/std/utilities/function.objects/range.cmp/less.pass.cpp:26
+struct NotTotallyOrdered {
+  bool operator<(NotTotallyOrdered) { return true; }
+};
----------------
Use hidden friends; they're harder to screw up. Here I assume leaving off the `const` was accidental, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100429