Skip to content
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

[class.compare] Don't introduce V for the return value #6035

Merged
merged 2 commits into from Nov 10, 2023

Conversation

brevzin
Copy link
Contributor

@brevzin brevzin commented Dec 19, 2022

In both cases, I'm not sure introducing V helps much - just requires name lookup for V. If we just say "the return value" in every case, I think that's clearer.

In both cases, I'm not sure introducing `V` helps much - just requires name lookup for `V`. If we just say "the return value" in every case, I think that's clearer.
@leni536
Copy link

leni536 commented Dec 20, 2022

See also #6036 for some additional considerations for improving the wording here.

@jensmaurer
Copy link
Member

This is a clear improvement, regardless of where #6036 goes.

@tkoeppe
Copy link
Contributor

tkoeppe commented Dec 28, 2022

I'm not sure it's a "clear improvement": both versions seem equally reasonable, and one could make the case that a repeated concept should be named. I don't find this a super strong case, but am happy to hear others' position. Thoughts?

@brevzin
Copy link
Contributor Author

brevzin commented Dec 28, 2022

one could make the case that a repeated concept should be named

One could, I guess, but the repeated concept in this case is only three words ("the return value"), is a very simple concept, and it is repeated only one time.

source/classes.tex Outdated Show resolved Hide resolved
source/classes.tex Outdated Show resolved Hide resolved
Per suggestions
@tkoeppe tkoeppe self-requested a review November 10, 2023 00:44
Copy link
Contributor

@tkoeppe tkoeppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine.

@tkoeppe tkoeppe merged commit 78300aa into cplusplus:main Nov 10, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants