New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[string.view], [string.view.comparison] Refactor string_view comparis… LWG3950 #6324
[string.view], [string.view.comparison] Refactor string_view comparis… LWG3950 #6324
Conversation
…ons, exploiting rewritten candidates With the current semantics of rewritten candidates for the comparison operators, it is superfluous to specify both overloads ``` operator==(basic_string_view, basic_string_view) operator==(basic_string_view, type_identity_t<basic_string_view>) ``` (ditto for `operator<=>`). The second overload is necessary in order to implement the "sufficient additional overloads" part of [string.view.comparison], but it is also sufficient, as all the following cases * `sv == sv` * `sv == convertible_to_sv` * `convertible_to_sv == sv` can in fact use it (directly, or after being rewritten with the arguments swapped). The reason why we still do have both operators seems to be historical; there is an explanation offered here https://stackoverflow.com/a/70851101 . Basically, there were _3_ overloads before a bunch of papers regarding `operator<=>` and `operator==` were merged: 1. `op==(sv, sv)` to deal with `sv == sv`; 2. `op==(sv, type_identity_t<sv>)` and 3. `op==(type_identity_t<sv>, sv)` to deal with `sv == convertible` and viceversa. Overload n.1 was necessary because with only 2. and 3. a call like `sv == sv` would be ambiguous. With the adoption of the rewriting rules, overload n.3 has been dropped, without realizing that overload n.1 would then become redundant. This commit removes `op==(sv,sv)` from the note that suggests how to implement the "sufficient additional overloads". I've left the overload in the synopsis untouched, however, under the assumption that its presence there doesn't mean that an implementation *must* provide it.
Alternative to #6270. |
I don't think that's a valid assumption ... although users are forbidden from taking the address of that function now, so I don't immediately see how they'd observe whether it's present. Either way, I dislike having declarations in a synopsis that are superfluous or just decorative. |
I don't know if I'd call it "decorative", I somehow thought that I needed it as an "anchor" to define the semantics of |
The semantics of equality comparison are defined by [string.view.comparison], not by non-existent declarations in the header synopsis. If a single So I would:
Does the same argument apply for |
OK. I had the impression that the wording deliberately didn't specify If all of this i just a someho silly pretense and there's no real reason not to specify it with
It does.
That's the idea. OK, I'll package something like this and send it over as a LWG defect, because now it sounds way bigger than editorial. |
|
Hello, This is now https://cplusplus.github.io/LWG/issue3950 , I'm not sure if LWG likes the approach, the reflector thread was a bit inconclusive? |
LWG might be able to look at it today. |
Ah OK, but if you have an LWG issue, then we don't need this one. Thanks! |
I mean, feel free to cherry pick the commit at a later time, if it implements the desired resolution. :-) A slightly different approach is in #6270 . I guess that can be closed as well for the same reason (not editorial)? |
Thanks, that's a nice thought, though it's just a little bit impractical given that we approve issue resolutions as a single, large paper and then draft edits on the back of that. So the important information would be to inform whoever is drafting the edits at that point that this PR already exists. I think for larger changes that could definitely be a nice bit of help (and we need to make sure to make a note on, say, the motions issue), but in this simple case it's probably not going to matter much. @Dani-Hub, @jwakely: Would it be possible to annoate the LWG issue with a note that wording exists already, so that we remember when the time comes to draft? |
done |
…ons, exploiting rewritten candidates
With the current semantics of rewritten candidates for the comparison operators, it is superfluous to specify both overloads
(ditto for
operator<=>
).The second overload is necessary in order to implement the "sufficient additional overloads" part of [string.view.comparison], but it is also sufficient, as all the following cases
sv == sv
sv == convertible_to_sv
convertible_to_sv == sv
can in fact use it (directly, or after being rewritten with the arguments swapped).
The reason why we still do have both operators seems to be historical; there is an explanation offered here https://stackoverflow.com/a/70851101 .
Basically, there were 3 overloads before a bunch of papers regarding
operator<=>
andoperator==
were merged:op==(sv, sv)
to deal withsv == sv
;op==(sv, type_identity_t<sv>)
andop==(type_identity_t<sv>, sv)
to deal withsv == convertible
and viceversa.Overload n.1 was necessary because with only 2. and 3. a call like
sv == sv
would be ambiguous. With the adoption of the rewriting rules, overload n.3 has been dropped, without realizing that overload n.1 would then become redundant.This commit removes
op==(sv,sv)
from the note that suggests how to implement the "sufficient additional overloads". I've left the overload in the synopsis untouched, however, under the assumption that its presence there doesn't mean that an implementation must provide it.