[libcxx-commits] [PATCH] D108250: [libc++][spaceship] Implement std::tuple::operator<=>

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision as: Mordante.
Mordante added a comment.

LGTM!



================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/three_way.pass.cpp:73
+    }
+    // Mixed types with floating point, which compare partially ordered
+    {
----------------
mumbleskates wrote:
> Mordante wrote:
> > mumbleskates wrote:
> > > Mordante wrote:
> > > > It would be nice to also mix signed and unsigned types.
> > > I threw in some indexes with matched unsigned types and unsigned <=> floating point. We probably can't do signed <=> unsigned integral types because <=> isn't defined, and the synth-three-way fallback generates promoted compile time errorwarnings about comparison with mixed signs. Not sure if we need to add any tests for this in synth-three-way, I think it's probably fine.
> > I think it's good to add some of these tests, especially since it's rather new feature, not only in the library but also in the compilers. The warnings can be disabled by adding `// ADDITIONAL_COMPILE_FLAGS: -Wno-sign-compare` at the beginning of the file.
> I've added that test since it's small, but I think it is more important and useful to add these tests to synth-three-way directly, where the behavior is implemented (D107721), rather than wracking our brains anew about all the corner cases each time we use the functionality. I've also added the test to the parent differential above as mentioned.
I see you added more detailed tests in D107721, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108250