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

Michael Schellenberger Costa via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jun 1 10:46:11 PDT 2020

miscco added a subscriber: BillyONeal.
miscco added inline comments.

Comment at: libcxx/include/__string:61
+#if __cplusplus > 201703L && defined(__cpp_impl_three_way_comparison) && __cpp_impl_three_way_comparison >= 201907
+#include <compare>
cjdb wrote:
> 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`?
I thought that is defined as:

#if !defined(__cpp_impl_three_way_comparison)

Which honestly should be essentially the same

Comment at: libcxx/include/compare:250
+  friend constexpr bool operator==(partial_ordering, partial_ordering) noexcept = default;
cjdb wrote:
> 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.
In the worst case we improved the consitency with all other methods.

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);
While I personally am in favor of reusing code I want to note that this carries some nonzero cost of creating a string_view.

This will almost certainly be optimized aways in RELEASE but will be noticeable in DEBUG.

I am kind of curios what the maintainers think about this. I know @BillyONeal avoids these miniscule pessimizations like the plague.



More information about the libcxx-commits mailing list