[libcxx-commits] [PATCH] D80891: [libcxx] adds operator<=> for basic_string_view

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/string_view:615
 {
-    if ( __lhs.size() != __rhs.size()) return false;
-    return __lhs.compare(__rhs) == 0;
+    return __lhs == basic_string_view<_CharT, _Traits>(__rhs);
 }
----------------
Why are we creating a copy of the string view at all?

Isn't the point of this overload to create a string view in the conversion of the argument to the parameter type?

That is, `common_type_t<basic_string_view<_Char, _Traits>> == basic_string_view<_Char, _Traits>`, no?


================
Comment at: libcxx/include/string_view:625
+    const auto result = __lhs.compare(__rhs) <=> 0;
+    if constexpr (requires { typename _Traits::comparison_category; }) {
+        return static_cast<typename _Traits::comparison_category>(result);
----------------
Please don't introduce a dependency on concepts here.

Instead, use something like:

```
template <class _Tp>
using __test_for_comparison_category = typename _Tp::comparison_category;

if constexpr ( _IsValidExpansion<__test_for_comparison_category, _Traits>::value ) {
```


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

https://reviews.llvm.org/D80891