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

Michael Schellenberger Costa via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jun 14 23:58:17 PDT 2020

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?



More information about the libcxx-commits mailing list