[libc++] Implement `operator<=>` for `error_category`

[avogelsgesang]

Summary

Implements part of P1614R2 "The Mothership has Landed"

[avogelsgesang]

Update review link

[mumbleskates]

libcxx/include/system_error

37

nit, per most-common pattern and personal preference

227

nit: Most checks of _LIBCPP_STD_VER are in terms of >, such that these blocks would be reversed, with C++20-onwards behavior gated behind _LIBCPP_STD_VER > 17. For consistency's sake I think it would be good to match that.

[avogelsgesang]

libcxx/include/system_error

227

I usually prefer it this way around, because I value "keeping the order between synopsis and implementation consistent" over "consistently use _LIBCPP_STD_VER > 17".

Do you think swapping the #if here is worth it, although that would mean that the order in the synopsis no longer matches the order in the implementation?

(related: https://reviews.llvm.org/D131372#inline-1263658)

[mumbleskates]

libcxx/include/system_error

227

okay, i can see that rationale. in my previous differentials (<=> for pair and tuple) i implemented it in the order you've seen me do, preferring a > 17 check and putting modern operators (==, <=>) first and outdated ones in the #else branch in cases where <=> obsoletes the other operators.

understanding your rationale i have a weaker preference now, still leaning towards whatever our conventional momentum is.

[avogelsgesang]

libcxx/include/system_error

227

@philnik @Mordante @ldionne @var-const does anyone of you have a preference here? Not sure which (purely stylistic) tradeoff we want to take here...

[Mordante]

LGTM modulo some nits.

libcxx/include/system_error

227

I have a preference, but we don't a required style. Traditionally the > xx was more common.

237

238

This requires including __memory/addressof.h. This guards against overloading operator&, not a real issue here but in other places, like containers it is.

[avogelsgesang]

address comments. Will merge as soon as CI finished

libcxx/include/system_error

227

I have a preference

You didn't mention what preference that is? I assume it is > xx, i.e. in line with the "traditional" style?

I updated the review with to use >xx now

[Mordante]

libcxx/include/system_error

227

I didn't mention it since code reviews shouldn't be about getting the code in the exact preferred style of the reviewer. Since we already use different styles there's no "project style".

But you guessed right and I personally prefer > XX, but a portion of that is just because I'm used to it.

libcxx/docs/Status/SpaceshipProjects.csv

 | `partial_order <https://reviews.llvm.org/D110738>`_
 | `strong_order_fallback <https://reviews.llvm.org/D111514>`_
 | `weak_order_fallback <https://reviews.llvm.org/D111514>`_
 | `partial_order_fallback <https://reviews.llvm.org/D111514>`_",None,Arthur O'Dwyer,|Complete| [#note-strongorder]_
 | `[alg.three.way] <https://wg21.link/alg.three.way>`_,| `lexicographical_compare_three_way <https://reviews.llvm.org/D80902>`_,[comparisons.three.way],Christopher Di Bella,|In Progress|
 | `[type.info] <https://wg21.link/type.info>`_,| `typeinfo <https://reviews.llvm.org/D130853>`_,None,Adrian Vogelsgesang,|Complete|
 | `[coroutine.handle.compare] <https://wg21.link/coroutine.handle.compare>`_,| `coroutine_handle <https://reviews.llvm.org/D109433>`_,[comparisons.three.way],Chuanqi Xu,|Complete|
 | `[pairs.spec] <https://wg21.link/pairs.spec>`_,| `pair <https://reviews.llvm.org/D107721>`_,[expos.only.func],Kent Ross,|Complete|
-| `[syserr.errcat.nonvirtuals] <https://wg21.link/syserr.errcat.nonvirtuals>`_,| error_category,[comparisons.three.way],Unassigned,|Not Started|
+| `[syserr.errcat.nonvirtuals] <https://wg21.link/syserr.errcat.nonvirtuals>`_,| `error_category <https://reviews.llvm.org/D131363>`_,[comparisons.three.way],Adrian Vogelsgesang,|Complete|
 | `[syserr.compare] <https://wg21.link/syserr.compare>`_,"| error_code
 | error_condition",None,Unassigned,|Not Started|
 | `[tuple.rel] <https://wg21.link/tuple.rel>`_,| `tuple <https://reviews.llvm.org/D108250>`_,[expos.only.func],Kent Ross,|Complete|
 "| `[optional.relops] <https://wg21.link/optional.relops>`_
 | `[optional.nullops] <https://wg21.link/optional.nullops>`_
 | `[optional.comp.with.t] <https://wg21.link/optional.comp.with.t>`_","| optional
 | nullopt",None,Kent Ross,|In Progress|
 "| `[variant.relops] <https://wg21.link/variant.relops>`_

libcxx/include/system_error

public:
 
     virtual const char* name() const noexcept = 0;
     virtual error_condition default_error_condition(int ev) const noexcept;
     virtual bool equivalent(int code, const error_condition& condition) const noexcept;
     virtual bool equivalent(const error_code& code, int condition) const noexcept;
     virtual string message(int ev) const = 0;
 
     bool operator==(const error_category& rhs) const noexcept;
-    bool operator!=(const error_category& rhs) const noexcept;
-    bool operator<(const error_category& rhs) const noexcept;
+    bool operator!=(const error_category& rhs) const noexcept;              // removed in C++20
+    bool operator<(const error_category& rhs) const noexcept;               // removed in C++20
+    strong_ordering operator<=>(const error_category& rhs) const noexcept;  // C++20
 };
 
 const error_category& generic_category() noexcept;
 const error_category& system_category() noexcept;
 
 template <class T> struct is_error_code_enum
     : public false_type {};
 
#endif
     virtual error_condition default_error_condition(int __ev) const _NOEXCEPT;
     virtual bool equivalent(int __code, const error_condition& __condition) const _NOEXCEPT;
     virtual bool equivalent(const error_code& __code, int __condition) const _NOEXCEPT;
     virtual string message(int __ev) const = 0;
 
     _LIBCPP_INLINE_VISIBILITY
     bool operator==(const error_category& __rhs) const _NOEXCEPT {return this == &__rhs;}
 
+#if _LIBCPP_STD_VER <= 17
+
     _LIBCPP_INLINE_VISIBILITY
     bool operator!=(const error_category& __rhs) const _NOEXCEPT {return !(*this == __rhs);}
 
     _LIBCPP_INLINE_VISIBILITY
     bool operator< (const error_category& __rhs) const _NOEXCEPT {return this < &__rhs;}
 
+#else // _LIBCPP_STD_VER <= 17
+
+    _LIBCPP_INLINE_VISIBILITY
+    strong_ordering operator<=>(const error_category& __rhs) const noexcept {return compare_three_way()(this, &__rhs);}
+
+#endif // _LIBCPP_STD_VER <= 17
+
     friend class _LIBCPP_HIDDEN __do_message;
 };
 
 class _LIBCPP_HIDDEN __do_message
     : public error_category
 {
 public:
     virtual string message(int __ev) const;

libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.nonvirtuals/cmp.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <system_error>
+
+// class error_category
+
+// strong_ordering operator<=>(const error_category& rhs) const noexcept;
+
+#include <system_error>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_comparisons.h"
+
+int main(int, char**) {
+  AssertOrderAreNoexcept<std::error_category>();
+  AssertOrderReturn<std::strong_ordering, std::error_category>();
+
+  const std::error_category& e_cat1 = std::generic_category();
+  const std::error_category& e_cat2 = std::generic_category();
+  const std::error_category& e_cat3 = std::system_category();
+
+  assert(testOrder(e_cat1, e_cat2, std::strong_ordering::equal));
+
+  bool isLess = e_cat1 < e_cat3;
+  assert(testOrder(e_cat1, e_cat3, isLess ? std::strong_ordering::less : std::strong_ordering::greater));
+
+  return 0;
+}