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

Rephrase "possibly const value" in terms of types #1563

Merged
merged 2 commits into from Sep 9, 2022

Conversation

jwakely
Copy link
Member

@jwakely jwakely commented Mar 20, 2017

Fixes #1497

source/containers.tex Outdated Show resolved Hide resolved
@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 2, 2017

@zygoloid: Should/can we adopt this change? Seems like a useful improvement.

source/containers.tex Outdated Show resolved Hide resolved
@zygoloid
Copy link
Member

I'm definitely happy to merge all but the first change in containers.tex. For that one change, I'm a little concerned that we're papering over a (pre-existing) wording bug.

@zygoloid zygoloid added the lwg Issue must be reviewed by LWG. label Nov 7, 2017
@jensmaurer
Copy link
Member

jensmaurer commented Nov 7, 2017

@jwakely reports that Marshall Clow is opposed to changing "const value of type X". Editorial meeting consensus: This is technically wrong, though. Interact with LWG.

@jwakely jwakely closed this Nov 23, 2017
@jwakely jwakely deleted the possibly-const-value branch November 23, 2017 20:12
@jwakely jwakely restored the possibly-const-value branch November 23, 2017 21:16
@jwakely
Copy link
Member Author

jwakely commented Nov 23, 2017

Sorry for the accidental close.

@jwakely jwakely reopened this Nov 23, 2017
@zygoloid zygoloid force-pushed the master branch 2 times, most recently from e3dbfe2 to 1a21a65 Compare July 7, 2018 23:19
@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Mar 12, 2020
@zygoloid zygoloid removed this from the C++20 milestone Mar 12, 2020
@jwakely jwakely force-pushed the possibly-const-value branch 2 times, most recently from 3de1933 to c69af47 Compare April 20, 2022 09:47
@jwakely jwakely removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Apr 20, 2022
Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Looks mostly good.

@tkoeppe , please have a look.

source/containers.tex Outdated Show resolved Hide resolved
@jwakely
Copy link
Member Author

jwakely commented Sep 5, 2022

I didn't think the Tentatively Ready poll for 3028 passed, but I've just done recount and it has five votes. But that won't be resolved until the next plenary now, sorry.

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 5, 2022

Oh OK, but in that case we probably won't need this PR, either, since it'll come back in a paper? But maybe we can reuse the edits here.

@jwakely
Copy link
Member Author

jwakely commented Sep 5, 2022

I don't think there's going to be a paper making these changes. I'll rebase it to remove the part that conflicts with 3028, so we can make the non-conflicting editorial changes.

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 5, 2022

Oh, I see, thanks! Let me know when it's ready.

@tkoeppe tkoeppe removed the lwg Issue must be reviewed by LWG. label Sep 5, 2022
@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 5, 2022

So is this editorial now? I'll remove the "lwg" label.

@jwakely
Copy link
Member Author

jwakely commented Sep 5, 2022

Yes, I think this is purely editorial, and as LWG 3028 shows, LWG is favouring putting "const" on the type, not the value, which is the subject of #1497 and this pull request.

I've rebased and removed the conflicts with LWG 3028. That means this won't entirely fix #1497, because the Tentatively Ready resolution for 3028 includes:

- - `r` denotes a non-const value of type `X`, and
+ - `s` and `t` denote non-const lvalues of type `X`, and
  - `rv` denotes a non-const rvalue of type `X`.

i.e. it changes one instance of "non-const value" to "non-const lvalues" and doesn't touch an instance of "non-const rvalue". Those should be fixed after 3028 is applied to the draft, instead of doing it as part of this pull request, to avoid invalidating the 3028 resolution that should get approved at the next plenary. Does that sound reasonable?

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 5, 2022

Sounds good! We can keep this PR open after merging as a tracking issue.

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 8, 2022

@jensmaurer, @zygoloid: would you care to provide a corish opinion and assent?

@jensmaurer
Copy link
Member

"value of type X or const X" is certainly in-your-face obvious and looks good to me.

off-topic: I wish we had some shorthand along the lines of "value of type cv X" or so.

@tkoeppe tkoeppe merged commit 88e574e into cplusplus:main Sep 9, 2022
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.

"a possibly const value of type X"
4 participants