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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
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>
----------------
Mordante wrote:
> 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?
I would like us to file a LWG issue and see what the answer is *before* we implement these in terms of hidden friends. I understand the benefits and I probably agree, but I don't want us to do something if the Standard is going to go a different way, however wrong we might think the Standard is.

Until there's a LWG issue that has been accepted and the feeling in the room seems to be "yeah, let's make that change", I'd rather hold off with this patch.


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

https://reviews.llvm.org/D114912