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
Fixed up several wording and variable naming issues in requirements tables #334
Conversation
Ping. Is there anybody who can take a look at this? |
Ping. |
I think the intention is that |
Thanks. Would you be happy if I simply made the change to "lvalues of type |
That would then be inconsistent with the previous row. I'd like to get the project editor's opinion before changing it. There might be good reasons that row has been unchanged since the 1998 standard, despite adding a row below it. Or there might be no good reason. I don't know. |
Sounds good. How do we go about doing that? |
I'd like "value of type X&" to be changed to "lvalue of type X", but please do so throughout; there are two others:
Note that the Regarding renaming the variable in the requirement... this is a horrible mess:
I think we should:
|
f7cc267
to
3658162
Compare
It doesn't make sense to re-use values of type X& when declaring a new X, as in the use of these variables in Table 28.
3658162
to
89f1e73
Compare
Thanks, Richard. I think I've hit everything you mentioned, and also deleted |
\tcode{a, a1, a2} & values of type \tcode{X\&} \\ \rowsep | ||
\tcode{a3} & an rvalue of type \tcode{X} \\ \rowsep | ||
\tcode{a, a1, a2} & lvalues of type \tcode{X} \\ \rowsep | ||
\tcode{u} & denotes the name of a variable being declared \\ \rowsep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the 'denotes' here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Thanks. @jwakely, I'd appreciate if you could take a look through this and make sure it all looks right to you, especially with an eye to library consistency and any subtle implications that LWG might be making with their choice of placeholder names. I'll also have another look through this before we move ahead with it. [One other thing: the use of 'value' versus 'lvalue' here has a category error; a 'value' is a result produced by evaluating an expression whereas an 'lvalue' is a kind of expression. I don't think this change reduces consistency, precision, or comprehensibility compared to the prior wording, but I think we can do better here. Equally, |
Yes, we should really do better at that kind of thing throughout the library. I'll take a look at the updated patch asap. |
In other tables of requirements, the identifier 'u' is used for variables being declared. Made that the case here, too. This necessitated a reflow of other variable names.
In other tables of requirements, the identifier 'u' is used for variables being declared. Made that the case here, too.
In other tables of requirements, the identifier 'u' is used for variables being declared. Made that the case here, too.
Both internally (it was inconsistent in "value" vs. "object") and externally (Tables 100 and 102 use "value"). While I was at it, fixed capitalization of the word "table" and fixed up TeX formatting.
915c187
to
e1c08b0
Compare
Thanks all. Let me know in detail what you'd like to do about 'value' and 'return type' and I'll be happy to do it. |
Ping. Do you mind pulling the commits as-is, assuming they are indeed an improvement, and then pursuing further wording changes separately? I don't mind sending follow-up pull requests. |
@jwakely: Ping. |
@jwakely: Ping. Can we close this out by using the comments as they are, then pursue larger changes later? |
@jwakely: Ping. :-/ |
I haven't forgotten about this, but last time I tried to merge it locally to review the rebuilt PDF I got several conflicts and didn't have time to resolve them because it's not hugely important. I'll look at it today and see if it can be merged. |
Wonderful, thank you! |
Table 27 in [allocator.requirements] defines variables a and a1 to have
type X&, so it doesn't make sense to re-use those names in Table 28 when
declaring a new variable of type X. Instead, use a name not used
elsewhere.