[libcxx-commits] [PATCH] D130295: [libc++] Uses operator<=> in string_view

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/string_view:805
+    basic_string_view<_CharT, _Traits> __lhs,
+    typename common_type<basic_string_view<_CharT, _Traits> >::type __rhs) noexcept {
+    if constexpr (requires { typename _Traits::comparison_category; })
----------------
Mordante wrote:
> Mordante wrote:
> > philnik wrote:
> > > jloser wrote:
> > > > Nit: can use `common_type_t` instead - here and elsewhere.
> > > I think this should actually be `type_identity_t`. You could theoretically have something like
> > > ```
> > > struct CharT {
> > >   // all the stuff to make it char-like
> > > };
> > > 
> > > struct CharTT {
> > >   template <class Traits>
> > >   CharTT(std::basic_string_view<CharT, Traits>);
> > >  };
> > > 
> > > template <class Traits>
> > > struct std::common_type<std::basic_string_view<CharT, Traits>, std::basic_string_view<CharT, Traits>> {
> > >   using type = CharTT;
> > > };
> > > ```
> > > right? That would result in a hard error.
> > > 
> > Good point.
> I'm not entirely clear what you mean, can you elaborate?
You are allowed to specialize `std::common_type` for types that depend on at least one user-defined type. That means that you could theoretically specialize it for the identity and return a different type. (`std::common_type<T>` forwards to `std::common_type<T, T>`) I think you actually want to `type_identity_t<basic_string_view<...>>` instead, because that guarantees that you get `basic_string_view<...>` back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130295