[libc++] [P1614] Hidden-friend operator<=> for string_view.

[ldionne]

Summary

This implements *almost* conforming behavior for string_view's operator<=>.
I'm making it a hidden friend so that we can get rid of the "additional sufficient overloads" that were needed to deal with implicit conversions when it was a free function template. The Microsoft ABI can't deal with having multiple templates that instantiate to the same signature like that. (Microsoft works around their ABI by adding defaulted template parameters to the "sufficient overloads" so that they mangle differently. We could also do that, but I'm agitating to keep this simple at the cost of corner-case conformance.)

N.B.: I know this is (ever so slightly) non-conforming; I'm claiming that libc++ should flex our vendor muscles and do it anyway, because the benefit is large and the cost is merely technical/pedantic.

The new test file is modeled on D114658.

[philnik]

libcxx/include/__string

334–336

Am I right that you use typedef instead of using for consistency here?

libcxx/include/string_view

713

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

2

Your formatting seems a bit inconsistent in this file.

96–102

I recommend not inheriting from standard types in general.

Is there any reason this does not apply here?

[• Quuxplusone]

libcxx/include/__string

334–336

Yep, just consistency with lines 329–333.

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

2

In what sense/where?
This is formatted the same as the rest of libcxx/test/std/strings/string.view/string.view.comparison/ (which I just landed recently), so anything I change in here I'll also change in those 6 files.

96–102

Sheer laziness. ;) See libcxx/test/support/constexpr_char_traits.h for the amount of stuff that would have to be repeated here: it's about 100 lines. I'm vocally willing to spend 10 or even 20 lines to avoid inheritance (and on the average iterator it's like 5 lines), but somewhere between 20 and 100 lines I lose some of my enthusiasm. :) Still, if someone shows how to eliminate this inheritance without adding 200 lines to this test, I'll be happy!

One idea to which I would be amenable is: Refactor libcxx/test/support/constexpr_char_traits.h into libcxx/test/support/test_char_traits.h, containing cpp17_char_traits<T> (which replaces constexpr_char_traits throughout the tests) and cpp20_char_traits<T, Ordering> (which replaces TraitsWithCompCat here, and in the tests for string::operator<=> once that's written). ... But even as I'm writing it out, it seems like a bad idea. cpp20_char_traits<T, Ordering> would be used in only 2 tests, so it doesn't pay for itself; and TraitsWithoutCompCat still needs to be written out longhand.

And I just realized that it might be nice to have another test for

struct TraitsWithUserDefinedCompCat : std::char_traits<char> {
    struct comparison_category {
        explicit comparison_category(std::strong_ordering);
    };
};

since for some reason C++20 doesn't require that comparison_category be a comparison category type.

[• Quuxplusone]

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

96–102

Update: What I wrote here doesn't work on the GCC buildkite anyway, because GCC and EDG have what-looks-like-a-bug: the compiler will "dig past" non-typenames to discover typenames in base classes. https://godbolt.org/z/qWxxoeE8r So I'll have to do something not-involving-inheritance here anyway! I don't have any good ideas what, though.

[jloser]

libcxx/include/__compare/comp_cat.h

28

What's _Dflt short for? Ditto below in the other alias templates.

34

Do we need to use void_t idiom if this is C++20-or-later code (can we use requires?)?

libcxx/include/string_view

29

Should we mention the note here that this is non-conforming? :)

757

We can probably drop the comment here given how it's just 6 lines up. Note in the next #if block below (#if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_RANGES)), we omit the comment since it's short like this one.

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

14

Nit: s/charT/CharT and s/traits/Traits to be consistent with libcxx code in string_view?

37

Since this test is run in C++20-or-later only, let's s/TEST_CONSTEXPR_CXX14/constexpr/g, please.

[philnik]

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

2

Ok, my wording was not exactly perfect. It's actually only the braces in lines 37 and 104. Everything else seems Ok.

[ldionne]

Can you show me how that's non-conforming? I think you've explained it elsewhere but can you post a link if you have one?

libcxx/include/__compare/comp_cat.h

28

I assume it means _Default. We should just use that name, it's clearer and those 3 additional characters don't cost a lot.

[• Quuxplusone]

libcxx/include/__compare/comp_cat.h

28

+1. I thought I remembered _Dflt being used elsewhere, but it's not; I see it only in the section heading [unique.ptr.dltr.dflt] and as a variable named __dflt in <locale>; meanwhile I see _Default in <experimental/type_traits>. I'll change it to _Default.

34

We have recently obsoleted _LIBCPP_HAS_NO_SPACESHIP_OPERATOR, but we still guard things behind _LIBCPP_HAS_NO_CONCEPTS, so, no, requires wouldn't be as generally useful, quite yet.

libcxx/include/string_view

29

And @ldionne wrote:

Can you show me how that's non-conforming? I think you've explained it elsewhere but can you post a link if you have one?

The difference between "free template" and "hidden-friend non-template" is detectable by the user: in particular, this PR makes three_way_comparable<reference_wrapper<string_view>> true, where in libstdc++ it's false. (I consider this a good thing, though!)
https://godbolt.org/z/YK6KaarqW
https://quuxplusone.github.io/blog/2021/10/22/hidden-friend-outlives-spaceship/

Re mentioning here, e.g. inserting the word non-conformingly before make: I'm leery of adding code comments that reflect gripes about the current state of the Standard (or guesses about its intent), because they tend to bit-rot over time. I'd definitely mention it in my commit message — commit messages are for the ephemeral "news" about a change at-the-time-it-was-made — but my default behavior is not to put it in permanent code comments. Someone who wants to know the historical background of this line can always git blame or git log the file.

757

Agreed; but this whole block is just moved (from old line 683), not modified, so I prefer to leave it as-was.

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

2

Ah: function braces uncuddled, class-definition braces cuddled. I see. (I was never gonna notice that on my own without that extra information. :)) I don't particularly care either way — I just did what came naturally to my fingers (which I'm sure is influenced by what I see, and vice versa what I type influences the ratios of what's checked in, over time). I don't think we have much of a consistent house style here — but both "function braces uncuddled" and "class-definition braces cuddled" seem to be in the numerical majority.

$ git grep 'int main(.*)$' libcxx/test/ | wc -l
    5353
$ git grep 'int main(.*) {' libcxx/test/ | wc -l
    1381
$ git grep 'int main(.*) {.*}$' libcxx/test/ | wc -l
     123

versus

$ git grep 'struct .* {$' libcxx/test/ | wc -l
    2277
$ git grep 'struct .*[a-zA-Z0-9_]$' libcxx/test/ | wc -l
    1171

So I'm inclined to do nothing here, unless a strong preference for some-specific-rule emerges.

14

IIRC we had a roughly isomorphic nit session (instigated by me ;)) about span::extent, where the template parameter is named Extent but exposed publicly and sometimes used in signatures as extent, which is inconsistent and annoying. Here, similarly, https://eel.is/c++draft/string.view#template spells the template parameters as charT, traits, and so sometimes we "correct" it to CharT, Traits and usually we don't. (And in a few tests' comments we say _CharT.) Anyway, charT is consistent with
libcxx/test/std/strings/string.view/string.view.io/ and almost all of libcxx/test/std/strings/basic.string/, among others, so I'm inclined to just leave it as-is.

[jloser]

libcxx/include/string_view

29

SGTM. I'm OK with it as a commit message only.

@ldionne the string_view comparison functions are in the "Non-member comparison functions" section (https://eel.is/c++draft/string.view.comparison). This is the same for some other class templates that have this "issue" (i.e. they could be written in terms of hidden friends, but aren't in the standard). The benefit gain for string_view is more-so than other class templates due to the additional two overloads per comparison operator for the "convertible to string_view in a non-deducible context" case. Those overloads go away if we just move to hidden friends.

757

Got it - thanks for explaining.

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.pass.cpp

14

Ah yes, I recall that now regarding using the value from the template or the static constexpr value from the span PR. I'll just sigh and say leave it as-is given the precedence. :)

Thanks for explaining.

[• Quuxplusone]

Review comments.
Eliminate inheritance from char_traits<char> (it wasn't that many lines after all, once I dropped unnecessary constexprness).
I'd missed "Mandates: R denotes a comparison category type," so now there's a static_assert with that message and a .verify.cpp test to verify the message.

[• Quuxplusone]

libcxx/include/string_view

713

Oops, missed this one. Have now fixed it locally, so it'll be in the next diff uploaded and/or landed.

[jloser]

libcxx/include/string_view

716

Is the message more useful for developers using libc++ or implementors? I suspect the former, in which case I would mention something about traits​::​comparison_­category rather than R. I claim R is not useful for developers as that requires them to look at the Standard to figure out R means traits​::​comparison_­category. WDYT?

[• Quuxplusone]

libcxx/include/string_view

716

Very fair point. I was kinda sneaking in the camel's nose of a new style, that whenever we insert a static_assert for literally no other reason than "the Standard says mandate this," we should make the message of the static_assert match the wording in the Standard. Which, if the reader is familiar with the Standard, should make it pretty obvious what's going on and why. But, you correctly observe that most readers won't be familiar with the Standard and so copying the Standard wording (while consistent (if we did it consistently)) isn't terribly helpful to the reader.

I don't mind changing this to either the more traditional

static_assert(__is_comparison_category_v<_Rp>, "Traits::comparison_category must be a comparison category type");

or the hybrid

static_assert(__is_comparison_category_v<_Rp>, "Mandates: Traits::comparison_category denotes a comparison category type");

Thoughts (from anyone at all)?

[jloser]

libcxx/include/string_view

716

I weakly prefer the hybrid:

static_assert(__is_comparison_category_v<_Rp>, "Mandates: Traits::comparison_category denotes a comparison category type");

I'd be fine with either of the two proposed different messages though.

[Mordante]

libcxx/include/string_view

29

About this remark and

I'm claiming that libc++ should flex our vendor muscles and do it anyway, because the benefit is large and the cost is merely technical/pedantic.

How about filing an LWG-issue and propose a fix. Then add comment here it implements LWG-xxxx and its proposed resolution?

714

[ldionne]

libcxx/include/string_view

29

I would like us to file a LWG issue and see what the answer is *before* we implement these in terms of hidden friends. I understand the benefits and I probably agree, but I don't want us to do something if the Standard is going to go a different way, however wrong we might think the Standard is.

Until there's a LWG issue that has been accepted and the feeling in the room seems to be "yeah, let's make that change", I'd rather hold off with this patch.

[• Quuxplusone]

libcxx/include/string_view

29

Can someone-not-me file that LWG issue, then?
I feel pretty strongly that WG21 is never actually going to take the plunge until there's a vendor willing to stand up and say "We do that and it has been fine," and asking the question at this point is just going to get an automatic "no." (Kind of like [[trivially_relocatable]]. ;)) So I'd like the most senior-official person possible to make that LWG-issue request (looking at you @ldionne)... and I'd also still like us to Just Do It in libc++ regardless of WG21's answer.

[ldionne]

libcxx/include/string_view

29

Can someone-not-me file that LWG issue, then?
I feel pretty strongly that WG21 is never actually going to take the plunge until there's a vendor willing to stand up and say "We do that and it has been fine," and asking the question at this point is just going to get an automatic "no."

I don't feel like that's the case. When libc++ (or someone who works on libc++) files an issue, usually I've seen it considered seriously, as it should be. Anyway, I don't mind doing it.

and I'd also still like us to Just Do It in libc++ regardless of WG21's answer.

We have a hard stance on not being non-conforming, so I don't feel comfortable doing it. Especially since it will make us *subtly* non-conforming, which means we might end up with more issues down the road. For example, imagine the Standard decides to use some clever SFINAE to enable some function based on whether x <=> y is valid for some x and y. If we are not strictly conforming, we could end up breaking that API too by our non-conformity, and then we're in a more complex situation. That's one of the reasons why I've been advocating a hard stance on 0 nonconformity whenever we can.

[Extensions are a different thing in some cases. I'm also against extensions, however some extensions can't really have bad interactions with other parts of the library so they are more acceptable. I'm thinking about stuff like the __int128_t for random distributions we just did or adding [[nodiscard]] to some functions -- I can't easily imagine how those would lead us into subtle trouble down the road. This here feels like exactly the kind of thing that is going to lead us into trouble down the road.]

[• Quuxplusone]

Rebased.

[ldionne]

[Github PR transition cleanup]

This was done by D130295 in a conforming way (AFAICT).

libcxx/include/CMakeLists.txt

set(files
   __chrono/calendar.h
   __chrono/duration.h
   __chrono/file_clock.h
   __chrono/high_resolution_clock.h
   __chrono/steady_clock.h
   __chrono/system_clock.h
   __chrono/time_point.h
   __compare/common_comparison_category.h
+  __compare/comp_cat.h
   __compare/compare_three_way.h
   __compare/compare_three_way_result.h
   __compare/is_eq.h
   __compare/ordering.h
   __compare/partial_order.h
   __compare/strong_order.h
   __compare/synth_three_way.h
   __compare/three_way_comparable.h

libcxx/include/__compare/comp_cat.h

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___COMPARE_COMP_CAT_H
+#define _LIBCPP___COMPARE_COMP_CAT_H
+
+#include <__config>
+#include <type_traits>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17
+
+// [string.view.comparison]
+// Let R denote the type Traits::comparison_category if that qualified-id is
+// valid and denotes a type; otherwise R is weak_ordering.
+
+template<class _Traits, class _Default, class = void>
+struct __comp_cat {
+    using type = _Default;
+};
+
+template<class _Traits, class _Default>
+struct __comp_cat<_Traits, _Default, typename __void_t<typename _Traits::comparison_category>::type> {
+    using type = typename _Traits::comparison_category;
+};
+
+template<class _Traits, class _Default>
+using __comp_cat_t = typename __comp_cat<_Traits, _Default>::type;
+
+#endif // _LIBCPP_STD_VER > 17
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___COMPARE_COMP_CAT_H

libcxx/include/__compare/ordering.h

private:
   _ValueT __value_;
 };
 
 inline constexpr strong_ordering strong_ordering::less(_OrdResult::__less);
 inline constexpr strong_ordering strong_ordering::equal(_OrdResult::__equiv);
 inline constexpr strong_ordering strong_ordering::equivalent(_OrdResult::__equiv);
 inline constexpr strong_ordering strong_ordering::greater(_OrdResult::__greater);
 
+// [cmp.categories.pre]
+// The types partial_ordering, weak_ordering, and strong_ordering
+// are collectively termed the comparison category types.
+
+template<class _Tp>
+inline constexpr bool __is_comparison_category_v = __one_of_v<_Tp, partial_ordering, weak_ordering, strong_ordering>;
+
 #endif // _LIBCPP_STD_VER > 17
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___COMPARE_ORDERING_H

libcxx/include/__string

 
 #include <__algorithm/copy.h>
 #include <__algorithm/copy_backward.h>
 #include <__algorithm/copy_n.h>
 #include <__algorithm/fill_n.h>
 #include <__algorithm/find_end.h>
 #include <__algorithm/find_first_of.h>
 #include <__algorithm/min.h>
+#include <__compare/ordering.h>
 #include <__config>
 #include <__functional/hash.h>     // for __murmur2_or_cityhash
 #include <__iterator/iterator_traits.h>
 #include <cstdint>     // for uint_least16_t
 #include <cstdio>      // for EOF
 #include <cstring>     // for memcpy
 #include <iosfwd>      // for streampos & friends
 #include <type_traits> // for __libcpp_is_constant_evaluated

 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char>
 {
     typedef char      char_type;
     typedef int       int_type;
     typedef streamoff off_type;
     typedef streampos pos_type;
     typedef mbstate_t state_type;
+#if _LIBCPP_STD_VER > 17
+    typedef strong_ordering comparison_category;
+#endif
 
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX14
     void assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
     static inline _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
             {return __c1 == __c2;}
     static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return (unsigned char)__c1 < (unsigned char)__c2;}
 

 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<wchar_t>
 {
     typedef wchar_t   char_type;
     typedef wint_t    int_type;
     typedef streamoff off_type;
     typedef streampos pos_type;
     typedef mbstate_t state_type;
+#if _LIBCPP_STD_VER > 17
+    typedef strong_ordering comparison_category;
+#endif
 
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX14
     void assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
     static inline _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 == __c2;}
     static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 

 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char8_t>
 {
     typedef char8_t        char_type;
     typedef unsigned int   int_type;
     typedef streamoff      off_type;
     typedef u8streampos    pos_type;
     typedef mbstate_t      state_type;
+#if _LIBCPP_STD_VER > 17
+    typedef strong_ordering comparison_category;
+#endif
 
     static inline constexpr void assign(char_type& __c1, const char_type& __c2) noexcept
         {__c1 = __c2;}
     static inline constexpr bool eq(char_type __c1, char_type __c2) noexcept
         {return __c1 == __c2;}
     static inline constexpr bool lt(char_type __c1, char_type __c2) noexcept
         {return __c1 < __c2;}
 

 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char16_t>
 {
     typedef char16_t       char_type;
     typedef uint_least16_t int_type;
     typedef streamoff      off_type;
     typedef u16streampos   pos_type;
     typedef mbstate_t      state_type;
+#if _LIBCPP_STD_VER > 17
+    typedef strong_ordering comparison_category;
+#endif
 
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX14
     void assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
     static inline _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 == __c2;}
     static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 

 template <>
 struct _LIBCPP_TEMPLATE_VIS char_traits<char32_t>
 {
     typedef char32_t       char_type;
     typedef uint_least32_t int_type;
     typedef streamoff      off_type;
     typedef u32streampos   pos_type;
     typedef mbstate_t      state_type;
+#if _LIBCPP_STD_VER > 17
+    typedef strong_ordering comparison_category;
+#endif
 
     static inline _LIBCPP_CONSTEXPR_AFTER_CXX14
     void assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
     static inline _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 == __c2;}
     static inline _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
         {return __c1 < __c2;}
 

libcxx/include/module.modulemap

module codecvt {
     export *
   }
   module compare {
     header "compare"
     export *
 
     module __compare {
       module common_comparison_category { private header "__compare/common_comparison_category.h" }
+      module comp_cat                   { private header "__compare/comp_cat.h" }
       module compare_three_way          { private header "__compare/compare_three_way.h" }
       module compare_three_way_result   { private header "__compare/compare_three_way_result.h" }
       module is_eq                      { private header "__compare/is_eq.h" }
       module ordering                   { private header "__compare/ordering.h" }
       module partial_order              { private header "__compare/partial_order.h" }
       module strong_order               { private header "__compare/strong_order.h" }
       module synth_three_way            { private header "__compare/synth_three_way.h" }
       module three_way_comparable       { private header "__compare/three_way_comparable.h" }

libcxx/include/string_view

namespace std {
 
     template<class charT, class traits>
     inline constexpr bool ranges::enable_view<basic_string_view<charT, traits>> = true;
 
     template<class charT, class traits>
     inline constexpr bool ranges::enable_borrowed_range<basic_string_view<charT, traits>> = true;  // C++20
 
     // 7.9, basic_string_view non-member comparison functions
+    // We make these hidden friends, in lieu of "sufficient additional overloads"
     template<class charT, class traits>
     constexpr bool operator==(basic_string_view<charT, traits> x,
                               basic_string_view<charT, traits> y) noexcept;
     template<class charT, class traits>
     constexpr bool operator!=(basic_string_view<charT, traits> x,
                               basic_string_view<charT, traits> y) noexcept;
     template<class charT, class traits>
     constexpr bool operator< (basic_string_view<charT, traits> x,
                                  basic_string_view<charT, traits> y) noexcept;
     template<class charT, class traits>
     constexpr bool operator> (basic_string_view<charT, traits> x,
                               basic_string_view<charT, traits> y) noexcept;
     template<class charT, class traits>
     constexpr bool operator<=(basic_string_view<charT, traits> x,
                                  basic_string_view<charT, traits> y) noexcept;
     template<class charT, class traits>
     constexpr bool operator>=(basic_string_view<charT, traits> x,
                               basic_string_view<charT, traits> y) noexcept;
-    // see below, sufficient additional overloads of comparison functions
 
     // 7.10, Inserters and extractors
     template<class charT, class traits>
       basic_ostream<charT, traits>&
         operator<<(basic_ostream<charT, traits>& os,
                    basic_string_view<charT, traits> str);
 
     // basic_string_view typedef names
namespace std {
   constexpr basic_string_view<char16_t> operator "" sv( const char16_t *str, size_t len ) noexcept;
   constexpr basic_string_view<char32_t> operator "" sv( const char32_t *str, size_t len ) noexcept;
 
 }  // namespace std
 
 
 */
 
+#include <__compare/comp_cat.h>
+#include <__compare/ordering.h>
 #include <__config>
 #include <__debug>
 #include <__ranges/concepts.h>
 #include <__ranges/data.h>
 #include <__ranges/enable_borrowed_range.h>
 #include <__ranges/enable_view.h>
 #include <__ranges/size.h>
 #include <__string>
#if _LIBCPP_STD_VER > 20
     bool contains(value_type __c) const noexcept
     { return find(__c) != npos; }
 
     constexpr _LIBCPP_INLINE_VISIBILITY
     bool contains(const value_type* __s) const
     { return find(__s) != npos; }
 #endif
 
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator==(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return (__lhs.size() == __rhs.size()) && (__lhs.compare(__rhs) == 0);
+    }
+
+#if _LIBCPP_STD_VER > 17
+    friend constexpr _LIBCPP_HIDE_FROM_ABI __comp_cat_t<_Traits, weak_ordering>
+    operator<=>(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        using _Rp = __comp_cat_t<_Traits, weak_ordering>;
+        static_assert(__is_comparison_category_v<_Rp>, "Mandates: R denotes a comparison category type");
+        return __lhs.compare(__rhs) <=> 0;
+    }
+#else
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator!=(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return (__lhs.size() != __rhs.size()) || (__lhs.compare(__rhs) != 0);
+    }
+
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator<(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return __lhs.compare(__rhs) < 0;
+    }
+
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator<=(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return __lhs.compare(__rhs) <= 0;
+    }
+
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator>(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return __lhs.compare(__rhs) > 0;
+    }
+
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI bool
+    operator>=(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        return __lhs.compare(__rhs) >= 0;
+    }
+#endif // _LIBCPP_STD_VER > 17
+
 private:
     const   value_type* __data;
     size_type           __size;
 };
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_RANGES)
 template <class _CharT, class _Traits>
 inline constexpr bool ranges::enable_view<basic_string_view<_CharT, _Traits>> = true;
 
 template <class _CharT, class _Traits>
 inline constexpr bool ranges::enable_borrowed_range<basic_string_view<_CharT, _Traits> > = true;
 #endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_RANGES)
 
 // [string.view.deduct]
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_RANGES)
 template <contiguous_iterator _It, sized_sentinel_for<_It> _End>
   basic_string_view(_It, _End) -> basic_string_view<iter_value_t<_It>>;
 #endif
 
-
 #if _LIBCPP_STD_VER > 20 && !defined(_LIBCPP_HAS_NO_RANGES)
 template <ranges::contiguous_range _Range>
   basic_string_view(_Range) -> basic_string_view<ranges::range_value_t<_Range>>;
 #endif
 
-// [string.view.comparison]
-// operator ==
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator==(basic_string_view<_CharT, _Traits> __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size()) return false;
-    return __lhs.compare(__rhs) == 0;
-}
-
-// The dummy default template parameters are used to work around a MSVC issue with mangling, see VSO-409326 for details.
-// This applies to the other sufficient overloads below for the other comparison operators.
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator==(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size()) return false;
-    return __lhs.compare(__rhs) == 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator==(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size()) return false;
-    return __lhs.compare(__rhs) == 0;
-}
-
-
-// operator !=
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator!=(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size())
-        return true;
-    return __lhs.compare(__rhs) != 0;
-}
-
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator!=(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size())
-        return true;
-    return __lhs.compare(__rhs) != 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator!=(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    if ( __lhs.size() != __rhs.size())
-        return true;
-    return __lhs.compare(__rhs) != 0;
-}
-
-
-// operator <
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) < 0;
-}
-
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) < 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) < 0;
-}
-
-
-// operator >
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator> (basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) > 0;
-}
-
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator>(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) > 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator>(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) > 0;
-}
-
-
-// operator <=
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<=(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) <= 0;
-}
-
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<=(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) <= 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator<=(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) <= 0;
-}
-
-
-// operator >=
-template<class _CharT, class _Traits>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator>=(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) >= 0;
-}
-
-
-template<class _CharT, class _Traits, int = 1>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator>=(basic_string_view<_CharT, _Traits> __lhs,
-                typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) >= 0;
-}
-
-template<class _CharT, class _Traits, int = 2>
-_LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator>=(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
-                basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
-{
-    return __lhs.compare(__rhs) >= 0;
-}
-
-
 template<class _CharT, class _Traits>
 basic_ostream<_CharT, _Traits>&
 operator<<(basic_ostream<_CharT, _Traits>& __os,
            basic_string_view<_CharT, _Traits> __str);
 
 // [string.view.hash]
 template<class _CharT>
 struct _LIBCPP_TEMPLATE_VIS hash<basic_string_view<_CharT, char_traits<_CharT> > >

libcxx/test/libcxx/diagnostics/detail.headers/compare/comp_cat.module.verify.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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__compare/comp_cat.h'}}
+#include <__compare/comp_cat.h>

libcxx/test/std/strings/string.view/string.view.comparison/spaceship.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
+
+// <string_view>
+
+// template<class charT, class traits>
+//   constexpr auto operator<=>(basic_string_view<charT, traits> lhs, basic_string_view<charT, traits> rhs);
+// (plus "sufficient additional overloads" to make implicit conversions work as intended)
+
+#include <string_view>
+#include <cassert>
+#include <compare>
+#include <string>
+
+#include "test_macros.h"
+#include "make_string.h"
+
+template<class T>
+struct ConvertibleTo {
+    T t_;
+    constexpr explicit ConvertibleTo(T t) : t_(t) {}
+    constexpr operator T() const {
+        return t_;
+    }
+};
+
+template<class SV, class OrderingT>
+constexpr bool test()
+{
+    typedef typename SV::value_type CharT;
+    typedef typename SV::traits_type Traits;
+
+    // Test the behavior of the operator, both with and without implicit conversions.
+    SV v[] = {
+        SV(MAKE_CSTRING(CharT, "")),
+        SV(MAKE_CSTRING(CharT, "abc")),
+        SV(MAKE_CSTRING(CharT, "abcdef")),
+        SV(MAKE_CSTRING(CharT, "acb")),
+    };
+    for (int i = 0; i < 4; ++i) {
+        for (int j = 0; j < 4; ++j) {
+            // See http://eel.is/c++draft/string.view#tab:string.view.comparison.overloads
+            std::strong_ordering expected = (i <=> j);
+            assert((v[i] <=> v[j]) == expected);
+            assert((v[i].data() <=> v[j]) == expected);
+            assert((v[i] <=> v[j].data()) == expected);
+            assert((ConvertibleTo<SV>(v[i]) <=> v[j]) == expected);
+            assert((v[i] <=> ConvertibleTo<SV>(v[j])) == expected);
+
+            ASSERT_SAME_TYPE(decltype(v[i] <=> v[j]), OrderingT);
+            ASSERT_SAME_TYPE(decltype(v[i].data() <=> v[j]), OrderingT);
+            ASSERT_SAME_TYPE(decltype(v[i] <=> v[j].data()), OrderingT);
+            ASSERT_SAME_TYPE(decltype(ConvertibleTo<SV>(v[i]) <=> v[j]), OrderingT);
+            ASSERT_SAME_TYPE(decltype(v[i] <=> ConvertibleTo<SV>(v[j])), OrderingT);
+
+            if (!TEST_IS_CONSTANT_EVALUATED) {
+                // TODO FIXME: once P0980 "Making std::string constexpr" is implemented
+                assert((std::basic_string<CharT, Traits>(v[i]) <=> v[j]) == expected);
+                assert((v[i] <=> std::basic_string<CharT, Traits>(v[j])) == expected);
+            }
+        }
+    }
+
+    // Test its behavior with embedded null bytes.
+    SV abc = SV(MAKE_CSTRING(CharT, "abc"));
+    SV abc0def = SV(MAKE_CSTRING(CharT, "abc\0def"), 7);
+    SV abcdef = SV(MAKE_CSTRING(CharT, "abcdef"));
+    assert((abc <=> abc0def) == std::strong_ordering::less);
+    assert((abc <=> abcdef) == std::strong_ordering::less);
+    assert((abc0def <=> abc) == std::strong_ordering::greater);
+    assert((abc0def <=> abcdef) == std::strong_ordering::less);
+    assert((abcdef <=> abc) == std::strong_ordering::greater);
+    assert((abcdef <=> abc0def) == std::strong_ordering::greater);
+
+    assert((abc.data() <=> abc0def) == std::strong_ordering::less);
+    assert((abc0def <=> abc.data()) == std::strong_ordering::greater);
+
+    if (!TEST_IS_CONSTANT_EVALUATED) {
+        // TODO FIXME: once P0980 "Making std::string constexpr" is implemented
+        assert((std::basic_string<CharT, Traits>(abc) <=> abc0def) == std::strong_ordering::less);
+        assert((abc0def <=> std::basic_string<CharT, Traits>(abc)) == std::strong_ordering::greater);
+    }
+
+    return true;
+}
+
+struct TraitsBase {
+    using char_type = char;
+    using int_type = int;
+    struct off_type {};
+    struct pos_type {};
+    struct state_type {};
+    static bool eq(char c, char d) { return c == d; }
+    static bool lt(char c, char d) { return c < d; }
+    static int compare(const char *p, const char *q, size_t n) { return memcmp(p, q, n); }
+    static size_t length(const char *p) { return strlen(p); }
+    static const char *find(const char *p, size_t n, char c) { return (const char*)memchr(p, c, n); }
+    static char *move(char *s, const char *p, size_t n) { return (char*)memmove(s, p, n); }
+    static char *copy(char *s, const char *p, size_t n) { return (char*)memcpy(s, p, n); }
+    static void assign(char& r, char d) { r = d; }
+    static char *assign(char *s, size_t n, char c) { return (char*)memset(s, c, n); }
+    static int not_eof(int e) { return e == -1 ? 0 : e; }
+    static char to_char_type(int e) { return e; }
+    static int to_int_type(char c) { return c; }
+    static bool eq_int_type(int e, int f) { return e == f; }
+    static int eof() { return -1; }
+};
+
+struct TraitsWithCompCat : TraitsBase {
+    using comparison_category = std::partial_ordering;
+};
+
+struct TraitsWithoutCompCat : TraitsBase {
+    void comparison_category();  // does not denote a type
+};
+
+int main(int, char**)
+{
+    test<std::string_view, std::strong_ordering>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+    test<std::wstring_view, std::strong_ordering>();
+#endif
+    test<std::u8string_view, std::strong_ordering>();
+    test<std::u16string_view, std::strong_ordering>();
+    test<std::u32string_view, std::strong_ordering>();
+    static_assert(test<std::string_view, std::strong_ordering>());
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+    static_assert(test<std::wstring_view, std::strong_ordering>());
+#endif
+    static_assert(test<std::u8string_view, std::strong_ordering>());
+    static_assert(test<std::u16string_view, std::strong_ordering>());
+    static_assert(test<std::u32string_view, std::strong_ordering>());
+
+    // Test that we forward the comparison category from the traits class.
+    test<std::basic_string_view<char, TraitsWithCompCat>, std::partial_ordering>();
+    test<std::basic_string_view<char, TraitsWithoutCompCat>, std::weak_ordering>();
+
+    return 0;
+}

libcxx/test/std/strings/string.view/string.view.comparison/spaceship_mandates.verify.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
+
+// <string_view>
+
+// template<class charT, class traits>
+//   constexpr R operator<=>(basic_string_view<charT, traits> lhs, basic_string_view<charT, traits> rhs);
+//   Mandates: R denotes a comparison category type.
+
+#include <string_view>
+#include <cassert>
+#include <compare>
+
+struct TraitsWithUserDefinedCompCat {
+    using char_type = char;
+    using int_type = int;
+    struct off_type {};
+    struct pos_type {};
+    struct state_type {};
+    static bool eq(char c, char d) { return c == d; }
+    static bool lt(char c, char d) { return c < d; }
+    static int compare(const char *p, const char *q, size_t n) { return memcmp(p, q, n); }
+    static size_t length(const char *p) { return strlen(p); }
+    static const char *find(const char *p, size_t n, char c) { return (const char*)memchr(p, c, n); }
+    static char *move(char *s, const char *p, size_t n) { return (char*)memmove(s, p, n); }
+    static char *copy(char *s, const char *p, size_t n) { return (char*)memcpy(s, p, n); }
+    static void assign(char& r, char d) { r = d; }
+    static char *assign(char *s, size_t n, char c) { return (char*)memset(s, c, n); }
+    static int not_eof(int e) { return e == -1 ? 0 : e; }
+    static char to_char_type(int e) { return e; }
+    static int to_int_type(char c) { return c; }
+    static bool eq_int_type(int e, int f) { return e == f; }
+    static int eof() { return -1; }
+
+    struct comparison_category {
+        comparison_category(std::strong_ordering o);
+        bool operator<(int) const;
+    };
+};
+
+void test1()
+{
+    std::basic_string_view<char, TraitsWithUserDefinedCompCat> sv = "hello";
+    (void)(sv == sv);  // OK
+    (void)(sv != sv);  // OK
+}
+
+void test2()
+{
+    std::basic_string_view<char, TraitsWithUserDefinedCompCat> sv = "hello";
+    (void)(sv < sv);  // expected-error@string_view:* {{Mandates: R denotes a comparison category type}}
+}