[libcxx-commits] [PATCH] D114912: [libc++] [P1614] Hidden-friend operator<=> for string_view.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/string_view:714
+    friend _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_HIDE_FROM_ABI __comp_cat_t<_Traits, weak_ordering>
+    operator<=>(basic_string_view __lhs, basic_string_view __rhs) _NOEXCEPT {
+        using _Rp = __comp_cat_t<_Traits, weak_ordering>;
----------------



================
Comment at: libcxx/include/string_view:29
     // 7.9, basic_string_view non-member comparison functions
+    // We make these hidden friends, in lieu of "sufficient additional overloads"
     template<class charT, class traits>
----------------
jloser wrote:
> Quuxplusone wrote:
> > jloser wrote:
> > > Should we mention the note here that this is non-conforming? :)
> > And @ldionne wrote:
> > > Can you show me how that's non-conforming? I think you've explained it elsewhere but can you post a link if you have one?
> > 
> > The difference between "free template" and "hidden-friend non-template" is detectable by the user: in particular, this PR makes `three_way_comparable<reference_wrapper<string_view>>` true, where in libstdc++ it's false. (I consider this a good thing, though!)
> > https://godbolt.org/z/YK6KaarqW
> > https://quuxplusone.github.io/blog/2021/10/22/hidden-friend-outlives-spaceship/
> > 
> > Re mentioning here, e.g. inserting the word `non-conformingly` before `make`: I'm leery of adding code comments that reflect gripes about the current state of the Standard (or guesses about its intent), because they tend to bit-rot over time. I'd definitely mention it in my //commit message// — commit messages are for the ephemeral "news" about a change at-the-time-it-was-made — but my default behavior is not to put it in permanent code comments. Someone who wants to know the historical background of this line can always `git blame` or `git log` the file.
> SGTM. I'm OK with it as a commit message only.
> 
> @ldionne the `string_view` comparison functions are in the "Non-member comparison functions" section (https://eel.is/c++draft/string.view.comparison). This is the same for some other class templates that have this "issue" (i.e. they could be written in terms of hidden friends, but aren't in the standard). The benefit gain for `string_view` is more-so than other class templates due to the additional two overloads per comparison operator for the "convertible to string_view in a non-deducible context" case. Those overloads go away if we just move to hidden friends.
About this remark and

> I'm claiming that libc++ should flex our vendor muscles and do it anyway, because the benefit is large and the cost is merely technical/pedantic.

How about filing an LWG-issue and propose a fix. Then add comment here it implements LWG-xxxx and its proposed resolution?


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

https://reviews.llvm.org/D114912