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

[Kent Ross via Phabricator via libcxx-commits]

mumbleskates added inline comments.


================
Comment at: libcxx/include/__variant/monostate.h:33
+
+inline _LIBCPP_INLINE_VISIBILITY
+constexpr strong_ordering operator<=>(monostate, monostate) noexcept { return strong_ordering::equal; }
----------------
avogelsgesang wrote:
> as I learned today in a different review, `_LIBCPP_INLINE_VISIBILITY` was superseded by `_LIBCPP_HIDE_FROM_ABI` (see https://libcxx.llvm.org/DesignDocs/VisibilityMacros.html#visibility-macros)
i've replaced all cases of `_LIBCPP_INLINE_VISIBILITY` with `_LIBCPP_HIDE_FROM_ABI` and removed `inline` in every case where the function itself was either `constexpr` or a template function.

this establishes consistency and modern practice at the expense of ~90 lines of noise. let me know if we ought to fork that to a new differential.


================
Comment at: libcxx/include/variant:184
   constexpr bool operator==(monostate, monostate) noexcept;
-  constexpr bool operator!=(monostate, monostate) noexcept;
+  constexpr bool operator!=(monostate, monostate) noexcept;             // until C++20
+  constexpr bool operator<(monostate, monostate) noexcept;              // until C++20
----------------
Mordante wrote:
> we typically use `remove in` instead of `until` (like cppreference).
i see 35 instances of "// until c++##" and 41 instances of "// removed in c++##" in v1/include. cppreference uses the two interchangeably, typically putting "until" next to the full signature (as we are writing here) and "removed in" next to the short name. (see: [pair/operator_cmp](https://en.cppreference.com/w/cpp/utility/pair/operator_cmp), [variant/operator_cmp](https://en.cppreference.com/w/cpp/utility/tuple/operator_cmp), [monostate](https://en.cppreference.com/w/cpp/utility/variant/monostate)).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131372