-
Notifications
You must be signed in to change notification settings - Fork 771
Define 'object type' only once. #1268
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
Conversation
source/basic.tex
Outdated
@@ -391,7 +391,7 @@ | |||
\item an object of type \tcode{T} is defined~(\ref{basic.def}), or | |||
\item a non-static class data member of type \tcode{T} is | |||
declared~(\ref{class.mem}), or | |||
\item \tcode{T} is used as the object type or array element type in a | |||
\item \tcode{T} is used as the object's type or array element type in a |
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.
It seems a little awkward to use the possessive in only one of the two cases here. Maybe "used as the allocated type or array element type" would be better?
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.
Yes, sounds better.
source/basic.tex
Outdated
\defn{const-volatile-qualified} version. The term | ||
\term{object type}~(\ref{intro.object}) includes the cv-qualifiers | ||
\defn{const-volatile-qualified} version. An | ||
object's type~(\ref{intro.object}) includes the cv-qualifiers |
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.
Here, perhaps "The type of an object" instead of "An object's type"?
While we're looking at this line, is there a reason why "cv-qualifiers" here isn't \grammarterm
'd? We definitely seem to be talking about the grammar element.
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.
Yes, we're talking about the grammar element, although the entire paragraph is odd:
using T = const int;
T x1;
const int x2;
const T x3;
All of the x1, x2, x3 objects have the same const-qualified type, even though there is no "const" cv-qualifier in the decl-specifier-seq of x1. But that fix is for another day.
I've added the "grammarterm" and change the possessive.
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.
Uh, our definition of \grammarterm has serious trouble handling
\grammarterm{cv-qualifier}s
(plural "s" after singular grammar non-terminal). There's lots of whitespace in between. That's also visible in 7.1.1 [dcl.stc] p5 (the note at the end; twice).
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.
Ah, we have
\newcommand{\grammarterm}[1]{\textit{#1}\xspace}
and the \xspace is probably the culprit (why do we need that in the first place?)
See also http://tex.stackexchange.com/questions/86565/drawbacks-of-xspace with the first answer from the package author of \xspace. Quote "So, if you find it useful, fine, it's there. But personally I wouldn't recommend it." |
I've added a separate commit to this pull request that removes \xspace from the definition of \grammarterm. A diffpdf in "appearance" mode shows 239 differences. I've looked through the first ones and found lots of noise and mildly reduced whitespace between a \grammarterm and a following punctuator as the only significant difference. For a full stop (period), this actually seems to improve matters; there was awkward extra spacing (just a tiny bit) between the r and the period in |
I'm not prepared to take a macro change the day before a working paper goes out unless someone has looked at all the resulting formatting changes; a survey of the first few is not sufficient at this point. I'll take this if either you've looked at all 239 and they are all OK, or if you change the "\grammarterm{cv-qualifier}s" to "\grammarterm{cv-qualifier}{s}", or even if you don't add the \grammarterm at all (since it was a drive-by anyway). I'm also a bit surprised we see any differences here in cases where \xspace was not adding a space; do we really have 239 bogus spaces inserted by this, or does \xspace also somehow change the formatting even when it doesn't add a space? If we don't make the macro change now, I'd still like to make it at some point. Generally I'd like for us to get rid of the \xspaces entirely. (See http://tex.stackexchange.com/questions/86565/drawbacks-of-xspace for the reason why, and note that even the original author of the package does not recommend it.) |
@zygoloid I wouldn't be surprised if |
@godbyk Thanks, that makes sense. I don't think that changes my position, though; I'd like to check that none of the 239 changes are losing a space that we want; that could happen pretty subtly:
... would I think lose the space after foo-name with this change. I can check this myself if no-one else does so first, but I have a few other things I need to get done before the pre-meeting mailing deadline. |
@zygoloid To clarify, my comment wasn't intended to change your position—I agree with it, given the inconsistencies I've seen in the code—but rather to provide an explanation for some of the minor changes you may see. |
There were only a few references to the definition in 1.8 [intro.object], so drop that definition and refer to 'type of an object' instead. Fixes cplusplus#511.
Updates:
|
There were only a few references to the definition in
1.8 [intro.object], so drop that definition and refer
to 'an object's type' instead.
Fixes #511.