[string.view], [string.view.comparison] Refactor string_view comparisons, exploiting rewritten candidates#6270
Closed
dangelog wants to merge 1 commit intocplusplus:mainfrom
Closed
Conversation
94efcba to
96f5958
Compare
…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` will, in fact, end up using 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 makes the operators in [string.view.comparison] exposition-only, and changes the implementation example for the "sufficient additional overloads" so that it only uses the strategy employing `type_identity_t`. Note: I didn't find a way to add a \expos without overrunning the right margin, so I renamed the section, like the exposition-only helpers in [mdspan].
96f5958 to
5343d0d
Compare
Contributor
|
I think this PR is misusing the term "exposition only". Can we only remove the first overload in the example? |
Contributor
Author
|
Hi, |
Contributor
Author
|
Done the minimal change only in #6324. |
Member
Then the superfluous one should be removed, not marked as exposition-only, or left in the synopsis and removed from a note. Either it's needed, or it's not. So either it should be there, or it shouldn't. I think this should be an LWG issue, not an editorial change. |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 == svsv == convertible_to_svconvertible_to_sv == svwill, in fact, end up using 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 == convertibleand viceversa.Overload n.1 was necessary because with only 2. and 3. a call like
sv == svwould 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 makes the operators in [string.view.comparison] exposition-only, and changes the implementation example for the "sufficient additional overloads" so that it only uses the strategy employing
type_identity_t.