[libc++] Extend __default_three_way_comparator to any types that only implements operator<=>

[philnik777]

This uses the new __builtin_lt_synthesises_from_spaceship builtin from clang to use three way comparison for arbitrary user-defined types that only provide a spaceship operator.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/157602.diff

3 Files Affected:

• (modified) libcxx/include/__utility/default_three_way_comparator.h (+23-3)
• (modified) libcxx/include/string (+2)
• (added) libcxx/test/libcxx/utilities/utility/has_default_three_way.compile.pass.cpp (+36)

diff --git a/libcxx/include/__utility/default_three_way_comparator.h b/libcxx/include/__utility/default_three_way_comparator.h
index ce423c6ce98e4..70eb1a27af11f 100644
--- a/libcxx/include/__utility/default_three_way_comparator.h
+++ b/libcxx/include/__utility/default_three_way_comparator.h
@@ -27,9 +27,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 template <class _LHS, class _RHS, class = void>
 struct __default_three_way_comparator;
 
-template <class _Tp>
-struct __default_three_way_comparator<_Tp, _Tp, __enable_if_t<is_arithmetic<_Tp>::value> > {
-  _LIBCPP_HIDE_FROM_ABI static int operator()(_Tp __lhs, _Tp __rhs) {
+template <class _LHS, class _RHS>
+struct __default_three_way_comparator<_LHS,
+                                      _RHS,
+                                      __enable_if_t<is_arithmetic<_LHS>::value && is_arithmetic<_RHS>::value> > {
+  _LIBCPP_HIDE_FROM_ABI static int operator()(_LHS __lhs, _RHS __rhs) {
     if (__lhs < __rhs)
       return -1;
     if (__lhs > __rhs)
@@ -38,6 +40,24 @@ struct __default_three_way_comparator<_Tp, _Tp, __enable_if_t<is_arithmetic<_Tp>
   }
 };
 
+#if _LIBCPP_STD_VER >= 20 && __has_builtin(__builtin_lt_synthesises_from_spaceship)
+template <class _LHS, class _RHS>
+struct __default_three_way_comparator<
+    _LHS,
+    _RHS,
+    __enable_if_t<(!is_arithmetic<_LHS>::value || !is_arithmetic<_RHS>::value) &&
+                  __builtin_lt_synthesises_from_spaceship(const _LHS&, const _RHS&)>> {
+  _LIBCPP_HIDE_FROM_ABI static int operator()(const _LHS& __lhs, const _RHS& __rhs) {
+    auto __res = __lhs <=> __rhs;
+    if (__res < 0)
+      return -1;
+    if (__res > 0)
+      return 1;
+    return 0;
+  }
+};
+#endif
+
 template <class _LHS, class _RHS, bool = true>
 inline const bool __has_default_three_way_comparator_v = false;
 
diff --git a/libcxx/include/string b/libcxx/include/string
index 0abdfebcb863f..f13a7640760f7 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2521,6 +2521,7 @@ _LIBCPP_STRING_V1_EXTERN_TEMPLATE_LIST(_LIBCPP_DECLARE, wchar_t)
 #  endif
 #  undef _LIBCPP_DECLARE
 
+#  if _LIBCPP_STD_VER <= 17 || !__has_builtin(__builtin_lt_synthesises_from_spaceship)
 template <class _CharT, class _Traits, class _Alloc>
 struct __default_three_way_comparator<basic_string<_CharT, _Traits, _Alloc>, basic_string<_CharT, _Traits, _Alloc> > {
   using __string_t _LIBCPP_NODEBUG = basic_string<_CharT, _Traits, _Alloc>;
@@ -2533,6 +2534,7 @@ struct __default_three_way_comparator<basic_string<_CharT, _Traits, _Alloc>, bas
     return __ret;
   }
 };
+#  endif
 
 #  if _LIBCPP_STD_VER >= 17
 template <class _InputIterator,
diff --git a/libcxx/test/libcxx/utilities/utility/has_default_three_way.compile.pass.cpp b/libcxx/test/libcxx/utilities/utility/has_default_three_way.compile.pass.cpp
new file mode 100644
index 0000000000000..b412e5a86f036
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/utility/has_default_three_way.compile.pass.cpp
@@ -0,0 +1,36 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+#include <__utility/default_three_way_comparator.h>
+#include <string>
+#include <vector>
+
+static_assert(std::__has_default_three_way_comparator_v<int, int>);
+static_assert(std::__has_default_three_way_comparator_v<int, long>);
+static_assert(std::__has_default_three_way_comparator_v<long, int>);
+static_assert(std::__has_default_three_way_comparator_v<long, long>);
+static_assert(std::__has_default_three_way_comparator_v<std::string, std::string>);
+
+#if __has_builtin(__builtin_lt_synthesises_from_spaceship)
+static_assert(std::__has_default_three_way_comparator_v<const std::string&, const std::string&>);
+static_assert(std::__has_default_three_way_comparator_v<const std::string&, const std::string_view&>);
+static_assert(std::__has_default_three_way_comparator_v<const std::string&, const char*>);
+static_assert(!std::__has_default_three_way_comparator_v<const std::string&, const wchar_t*>);
+
+static_assert(std::__has_default_three_way_comparator_v<const std::vector<int>&, const std::vector<int>&>);
+
+struct MyStruct {
+  int i;
+
+  friend auto operator<=>(MyStruct, MyStruct) = default;
+};
+
+static_assert(std::__has_default_three_way_comparator_v<const MyStruct&, const MyStruct&>);
+#endif

[ldionne]

This is nice! LGTM with small comments (and green CI).

Ah, also, it would be nice to check whether this has any impact on our benchmarks.

[ldionne]

It's nice that the builtin basically removes the need for this special case.

[ldionne]

This one can be taken out of the #if since we provide it even when the builtin isn't available.

[philnik777]

We're providing one with string, string, but not with const string&, const string&. It currently doesn't make a difference either, since we instantiate never instantiate __default_three_way_comparator with qualified types. This is more a "the builtin version is way more robust" test than anything else.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.