[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
Tue Jun 9 13:48:37 PDT 2020

cjdb added inline comments.

Comment at: libcxx/include/__string:602
     typedef mbstate_t      state_type;
+    #if __cplusplus > 201703L && defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201907
+    using comparison_category = strong_ordering;
EricWF wrote:
> What's the reason for the different preprocessor guards?
I think I didn't know about `_LIBCPP_HAS_NO_SPACESHP_OPERATOR` when I was writing these and forgot to change them all.

Comment at: libcxx/include/compare:368
+  _LIBCPP_INLINE_VISIBILITY friend constexpr bool operator==(weak_ordering, weak_ordering) noexcept = default;
EricWF wrote:
> Does this work if we don't have support for the spaceship operator?
Can `weak_ordering`, etc., even be available if we don't have spaceship support?

Comment at: libcxx/include/string_view:616
+    return __lhs == basic_string_view<_CharT, _Traits>(__rhs);
EricWF wrote:
> Aren't we losing the short circuit for strings of different lengths? 
> That optimization seems pretty important.
It is still present in `operator==(basic_string_view, basic_string_view)`, which is where the body of work happens. Do you want it put back into this "delegating" operator?

Alternatively, I can follow @miscco's note above. I left that unresolved as I felt it was pending further input.



More information about the libcxx-commits mailing list