[libcxx-commits] [PATCH] D81823: adds equality for spaceship types for themselves

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco accepted this revision.
miscco added a comment.
This revision is now accepted and ready to land.

This looks obviously correct.

Thanks for working on this. I have one small nit regarding whether we need all combinations for the test but feel free to ignore it. More tests are generally better that fewer.

NOTE: I am not a maintainer so you should hold on merging this until a maintainer greenlights it. That said this seem close to the `trivially correct` clause



================
Comment at: libcxx/include/compare:489
   _LIBCPP_INLINE_VISIBILITY friend constexpr bool operator>=(strong_ordering __v, _CmpUnspecifiedParam) noexcept;
   _LIBCPP_INLINE_VISIBILITY friend constexpr bool operator==(_CmpUnspecifiedParam, strong_ordering __v) noexcept;
   _LIBCPP_INLINE_VISIBILITY friend constexpr bool operator!=(_CmpUnspecifiedParam, strong_ordering __v) noexcept;
----------------
These are not part of the standard and should be handled by the rewriting of  the equality operator. But definitely not part of your PR. Just raising awareness


================
Comment at: libcxx/test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp:164
+
+    static_assert(std::weak_ordering::equivalent != std::weak_ordering::less);
+    static_assert(std::weak_ordering::equivalent ==
----------------
I am wondering if we really need `B != A` if we already checked `A != B` on the library side.

Can we trust the compiler to have its own valid tests?


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

https://reviews.llvm.org/D81823