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

Fixed up several wording and variable naming issues in requirements tables #334

Closed

Conversation

jacobsa
Copy link
Contributor

@jacobsa jacobsa commented Jun 20, 2014

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.

@jacobsa
Copy link
Contributor Author

jacobsa commented Jul 30, 2014

Ping. Is there anybody who can take a look at this?

@jacobsa
Copy link
Contributor Author

jacobsa commented Sep 10, 2014

Ping.

@jwakely
Copy link
Member

jwakely commented Sep 10, 2014

I think the intention is that a, a1 and a2 are lvalues of type X, and so using those names does make sense when declaring an lvalue of type X. I'm not sure why Table 27 says "values of type X&" rather than "lvalues of type X"

@jacobsa
Copy link
Contributor Author

jacobsa commented Sep 11, 2014

Thanks. Would you be happy if I simply made the change to "lvalues of type X"?

@jwakely
Copy link
Member

jwakely commented Sep 11, 2014

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.

@jacobsa
Copy link
Contributor Author

jacobsa commented Sep 11, 2014

Sounds good. How do we go about doing that?

@zygoloid
Copy link
Member

I'd like "value of type X&" to be changed to "lvalue of type X", but please do so throughout; there are two others:

source/lib-intro.tex:\tcode{t} & a value of type \tcode{const T&} \ \rowsep
source/lib-intro.tex:\tcode{a, a1, a2} & values of type \tcode{X&} \ \rowsep
source/lib-intro.tex:\tcode{r} & a value of type \tcode{T&}
source/lib-intro.tex:\tcode{s} & a value of type \tcode{const T&}

Note that the a3 row is redundant and should be removed -- a3 is not used by later requirements.

Regarding renaming the variable in the requirement... this is a horrible mess:

  • Table 28 uses a and says (in Table 27) that a is a value of type X&
  • Table 96 uses u and says "u denotes an identifier".
  • Table 99 uses u and says "u denotes a variable".
  • Table 100 (sequence container requirements) uses a and says that "a denotes a value of X". (This should also be changed to say "of type X", and there are more such occurrences to fix later in the same paragraph.)
  • Table 102 (associative container requirements) uses a and again says that "a denotes a value of X".
  • Table 103 (unordered associative container requirements) uses a and says "a is an object of type X", which is even worse than the "denotes a value of" wording. Ugh.

I think we should:

  • use u consistently as the name of a variable being declared in a requirements table
  • use "u denotes the name of a variable being declared" or similar
  • fix the places that say that "a denotes a value of X" to include the word "type"
  • fix the places that say "a is an object of type X" to use a more meaningful phrasing

@jacobsa jacobsa force-pushed the allocator-expression-variables branch 2 times, most recently from f7cc267 to 3658162 Compare September 18, 2014 01:18
@jacobsa jacobsa force-pushed the allocator-expression-variables branch from 3658162 to 89f1e73 Compare September 18, 2014 01:54
@jacobsa jacobsa changed the title Made the declarations in Table 28 use a distinct variable name. Fixed up several wording and variable naming issues in requirements tables Sep 18, 2014
@jacobsa
Copy link
Contributor Author

jacobsa commented Sep 18, 2014

Thanks, Richard. I think I've hit everything you mentioned, and also deleted
several more unused variables in Table 27. Please take another look.

\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
Copy link
Member

Choose a reason for hiding this comment

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

Remove the 'denotes' here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@zygoloid
Copy link
Member

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, T& is listed as a "return type" of an expression in various requirements tables, and that doesn't match the core language either; expressions don't have "return types", and the result type of an expression would not be a reference type. This seems like something LWG should consider more broadly.]

@jwakely
Copy link
Member

jwakely commented Sep 18, 2014

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.
@jacobsa jacobsa force-pushed the allocator-expression-variables branch from 915c187 to e1c08b0 Compare September 18, 2014 12:48
@jacobsa
Copy link
Contributor Author

jacobsa commented Sep 18, 2014

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.

@jacobsa
Copy link
Contributor Author

jacobsa commented Oct 6, 2014

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.

@jacobsa
Copy link
Contributor Author

jacobsa commented Nov 2, 2014

@jwakely: Ping.

@jacobsa
Copy link
Contributor Author

jacobsa commented Jan 26, 2015

@jwakely: Ping. Can we close this out by using the comments as they are, then pursue larger changes later?

@jacobsa
Copy link
Contributor Author

jacobsa commented Apr 14, 2015

@jwakely: Ping. :-/

@jwakely
Copy link
Member

jwakely commented Apr 14, 2015

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.

@jwakely
Copy link
Member

jwakely commented Apr 14, 2015

Merged as ea16476..1186a38

@jwakely jwakely closed this Apr 14, 2015
@jacobsa
Copy link
Contributor Author

jacobsa commented Apr 20, 2015

Wonderful, thank you!

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

3 participants