[libcxx-commits] [PATCH] D131372: Implement std::variant::operator<=>

[Adrian Vogelsgesang via Phabricator via libcxx-commits]

avogelsgesang added inline comments.


================
Comment at: libcxx/docs/Status/SpaceshipProjects.csv:28
 | `[variant.monostate.relops] <https://wg21.link/variant.monostate.relops>`_","| monostate
-| variant",None,Kent Ross,|In Progress|
+| variant",None,Kent Ross,|Complete|
 | `[unique.ptr.special] <https://wg21.link/unique.ptr.special>`_,| `unique_ptr <https://reviews.llvm.org/D130838>`_,[comparisons.three.way],Adrian Vogelsgesang,|Complete|
----------------
please also update `monostate` and `variant` to link to this review


================
Comment at: libcxx/include/__variant/monostate.h:34
+inline _LIBCPP_INLINE_VISIBILITY
+constexpr strong_ordering operator<=>(monostate, monostate) noexcept { return strong_ordering::equal; }
+
----------------
I usually try to keep the order between the synopsis and the implementation in sync.
In this case, the synopsis has `operator<=>` after the removed comparisons. I think we should also use that order here in the implementation, i.e. switch the `#if` branches


================
Comment at: libcxx/test/std/utilities/variant/variant.monostate.relops/relops.pass.cpp:26
+#if TEST_STD_VER > 17
+#include <compare>
+#endif // TEST_STD_VER > 17
----------------
afaik `<variant>` has a standard-mandated include to `<compare`. Hence, this include shouldn't be necessary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131372