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

[Kent Ross via Phabricator via libcxx-commits]

mumbleskates marked 2 inline comments as done.
mumbleskates added inline comments.


================
Comment at: libcxx/include/__compare/synth_three_way.h:1
+// THIS FILE NOT FOR REVIEW (see https://reviews.llvm.org/D107721 )
+//===----------------------------------------------------------------------===//
----------------
Mordante wrote:
> You can use the stack feature of Phabricator to make patch series. That makes reviewing the real changes easier.
Unfortunately not finding any clear descriptions of how to do this in any documentation and at this point I'm more afraid of making a huge mess in phabricator notifications, already bad enough with my personal rivalry with arcanist.


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.rel/three_way.pass.cpp:38
+        typedef std::tuple<> T0;
+        static_assert((T0() <=> T0()) == std::strong_ordering::equal, "");
+        // No member types yields strong ordering (all are equal).
----------------
Mordante wrote:
> The empty string in the `static_assert` is not required in C++20, please remove it.
TIL


================
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
+    {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108250