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

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jun 1 08:33:52 PDT 2020

cjdb added a comment.

Thanks @miscco!

Comment at: libcxx/include/__string:61
+#if __cplusplus > 201703L && defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201907
+#include <compare>
miscco wrote:
> I would suggest to create define like the other we usually find, e.g `_LIBCPP_STRING_HAS_NO_SPACESHIP_OPERATOR` (Note that they are usually negated)
> Then these could all be turned into 
Ack, I forgot to clean this one up. What's the advantage of `_LIBCPP_STRING_HAS_NO_SPACESHIP_OPERATOR` over `_LIBCPP_HAS_NO_SPACESHIP_OPERATOR`?

Comment at: libcxx/include/compare:250
+  friend constexpr bool operator==(partial_ordering, partial_ordering) noexcept = default;
miscco wrote:
> I guess this is missing `_LIBCPP_INLINE_VISIBILITY `
Does it need that? We've defined it here, so it should //already// have inline visibility.

Comment at: libcxx/include/string_view:783
+    const auto result = __lhs.compare(__rhs) <=> 0;
+    if constexpr (requires { typename _Traits::comparison_category; }) {
+        return static_cast<typename _Traits::comparison_category>(result);
miscco wrote:
> Out of curriosity. Do you have a working example where `result` `would *not* be the right comparison category?
For a live example, see lines 64-79 in the test case below.

I guess the reason it was designed this way is for backwards compatibility. Someone who designed their char_traits specialisation 20 years ago shouldn't be penalised for not having this trait.



More information about the libcxx-commits mailing list