[libc++] Fixes string_view comparison operators.

[Mordante]

Summary

While implementing operator<=> for string_view (D130295) @philnik
pointed out common_type should be type_identity. Since it was an
existing issue that wasn't addressed.

This addresses the issue for both the new and existing equality and
comparison operators. The test is based on the example posted in
D130295.

[huixie90]

LGTM

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

10

question : why !stdlib=libc++

[Mordante]

Thanks for the review!

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

10

libc++ provides string_view in all language Standards, other vendors only in C++17. So this disables the testing before C++17 when the library isn't libc++.

[philnik]

LGTM % nit.

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

13

36

You didn't have to keep my quick-n-diry implementation, but I guess it should be fine. Maybe add a static_assert(sizeof(char_wrapper) == 1) for my sanity? Unless it's guaranteed by the standard. In that case I'd be interested where that is guaranteed.

[Mordante]

Thanks for the reviews!

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

36

I'm sure it's guaranteed by the Standard, just not sure where ;-) But I'll add a static_assert.

libcxx/include/string_view

bool operator==(basic_string_view<_CharT, _Traits> __lhs,
     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
+                __type_identity_t<basic_string_view<_CharT, _Traits> > __rhs) _NOEXCEPT
 {
     if ( __lhs.size() != __rhs.size()) return false;
     return __lhs.compare(__rhs) == 0;
 }
 
 #if _LIBCPP_STD_VER < 20
 // This overload is automatically generated in C++20.
 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,
+bool operator==(__type_identity_t<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;
 }
 #endif // _LIBCPP_STD_VER > 17
 
 // operator <=>
if constexpr (requires { typename _Traits::comparison_category; }) {
         return static_cast<typename _Traits::comparison_category>(__lhs.compare(__rhs) <=> 0);
     } else {
         return static_cast<weak_ordering>(__lhs.compare(__rhs) <=> 0);
     }
 }
 
 template <class _CharT, class _Traits, int = 1>
 _LIBCPP_HIDE_FROM_ABI constexpr auto operator<=>(
-    basic_string_view<_CharT, _Traits> __lhs, common_type_t<basic_string_view<_CharT, _Traits>> __rhs) noexcept {
+    basic_string_view<_CharT, _Traits> __lhs, type_identity_t<basic_string_view<_CharT, _Traits>> __rhs) noexcept {
     if constexpr (requires { typename _Traits::comparison_category; }) {
         // [string.view]/4
         static_assert(
             __comparison_category<typename _Traits::comparison_category>,
             "return type is not a comparison category type");
         return static_cast<typename _Traits::comparison_category>(__lhs.compare(__rhs) <=> 0);
     } else {
         return static_cast<weak_ordering>(__lhs.compare(__rhs) <=> 0);
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
+                __type_identity_t<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 = 2>
 _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
-bool operator!=(typename common_type<basic_string_view<_CharT, _Traits> >::type __lhs,
+bool operator!=(__type_identity_t<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;
 }
 
 
 // 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
+                __type_identity_t<basic_string_view<_CharT, _Traits> > __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,
+bool operator<(__type_identity_t<basic_string_view<_CharT, _Traits> > __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
+                __type_identity_t<basic_string_view<_CharT, _Traits> > __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,
+bool operator>(__type_identity_t<basic_string_view<_CharT, _Traits> > __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
+                __type_identity_t<basic_string_view<_CharT, _Traits> > __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,
+bool operator<=(__type_identity_t<basic_string_view<_CharT, _Traits> > __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
+                __type_identity_t<basic_string_view<_CharT, _Traits> > __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,
+bool operator>=(__type_identity_t<basic_string_view<_CharT, _Traits> > __lhs,
                 basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT
 {
     return __lhs.compare(__rhs) >= 0;
 }
 
 #endif //  _LIBCPP_STD_VER > 17
 
 template<class _CharT, class _Traits>

libcxx/test/std/strings/string.view/string.view.comparison/common_type_specialization.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: !stdlib=libc++ && (c++03 || c++11 || c++14)
+
+// During the review D130295 it was noticed libc++'s implementation uses
+// std::common_type. When users specialize this template for their own types the
+// comparisions would fail. This tests with a specialized std::common_type.
+
+// <string_view>
+
+#include <string_view>
+
+#include <cassert>
+#include <cstring>
+
+#include "test_comparisons.h"
+#include "test_macros.h"
+
+struct char_wrapper {
+  char c;
+};
+
+template <>
+struct std::char_traits<char_wrapper> {
+  using char_type = char_wrapper;
+
+  static bool eq(char_wrapper lhs, char_wrapper rhs) { return lhs.c == rhs.c; }
+
+  static size_t length(const char_wrapper* a) {
+    static_assert(sizeof(char_wrapper) == 1, "strlen requires this");
+    return std::strlen(reinterpret_cast<const char*>(a));
+  }
+
+  static int compare(const char_wrapper* lhs, const char_wrapper* rhs, std::size_t count) {
+    return std::char_traits<char>::compare(
+        reinterpret_cast<const char*>(lhs), reinterpret_cast<const char*>(rhs), count);
+  }
+};
+
+using WrappedSV = std::basic_string_view<char_wrapper, std::char_traits<char_wrapper> >;
+
+// std::common_type can be specialized and not have a typedef-name member type.
+template <>
+struct std::common_type<WrappedSV, WrappedSV> {};
+
+struct convertible_to_string_view {
+  WrappedSV sv;
+  convertible_to_string_view(const char_wrapper* a) : sv(a) {}
+  operator WrappedSV() const { return sv; }
+};
+
+template <class T, class U>
+void test() {
+  char_wrapper a[] = {{'a'}, {'b'}, {'c'}, {'\0'}};
+
+  assert((testComparisons(T(a), U(a), true, false)));
+
+#if TEST_STD_VER > 17
+  assert((testOrder(T(a), U(a), std::weak_ordering::equivalent)));
+#endif
+}
+
+int main(int, char**) {
+  test<WrappedSV, convertible_to_string_view>();
+
+  return 0;
+}