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

Remove bad \grammarterm for "injected-class-name" #1555

Merged
merged 1 commit into from Apr 8, 2017

Conversation

timsong-cpp
Copy link
Contributor

No description provided.

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 18, 2017

Why so many commits -- squash and rebase?

@timsong-cpp
Copy link
Contributor Author

Figured separate commits might be easier to review, but OK, squashed.

@@ -672,8 +672,8 @@
class type \tcode{C}, the \grammarterm{unqualified-id}{s}
\tcode{begin} and \tcode{end} are looked up in the scope of \tcode{C}
as if by class member access lookup~(\ref{basic.lookup.classref}), and if either
(or both) finds at least one declaration, \grammarterm{begin-expr} and
\grammarterm{end-expr} are \tcode{__range.begin()} and \tcode{__range.end()},
(or both) finds at least one declaration, \textit{begin-expr} and
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't really want to introduce new \textits. Consult with @jensmaurer for details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every other occurrence of meow-expr in this section uses \textit, which is why I used them here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that. We still need to talk about this.

Copy link
Member

Choose a reason for hiding this comment

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

It seems to me begin-expr (and its friend end-expr) should be \placeholder, actually: It's a meta-variable (for a chunk of code, in this case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So \placeholder it throughout? That was admittedly my first thought, before seeing all-\textits in the surroundings.

Copy link
Member

Choose a reason for hiding this comment

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

See #1558.

@@ -2110,7 +2110,7 @@
attribute-specifier-seq\opt decl-specifier-seq\opt declarator virt-specifier-seq\opt{} \terminal{ = default ;}
\end{ncbnf}

is called an \grammarterm{explicitly-defaulted} definition.
is called an \term{explicitly-defaulted} definition.
A function that is explicitly defaulted shall
Copy link
Member

Choose a reason for hiding this comment

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

This should be \defn{explicitly-defaulted definition}, probably with "definition!explicitly-defaulted" as the main index entry and a "see" pointer for "explicitly-defaulted definition".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's already a \indextext{definition!function!explicitly-defaulted} ~10 lines above, which is why I went with \term.

Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, \term should only (if at all) survive for stuff that is local enough not to ever be referenced from elsewhere, and thus not needing an index entry at all. For this particular case, it seems the \indextext from before should simply be folded into the \defnx to be used after "is called an".

I'm slightly concerned that we seem to define "explicitly-defaulted definition" as the technical term, although we mean "explicitly-defaulted function definition". That probably needs a bit of research what we actually want here.

@jensmaurer
Copy link
Member

This needs a rebase.

@timsong-cpp
Copy link
Contributor Author

I dropped the explicitly-defaulted change; the indexing issue is above my pay grade. The only remaining change is injected-class-name.

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 25, 2017

@zygoloid: What do you think? It looks like the PR in its present form improves consistency.

@jensmaurer
Copy link
Member

Yes, it removes a bad \grammarterm.

@jensmaurer jensmaurer changed the title A few more \grammarterm fixes Remove bad \grammarterm for "injected-class-name" Mar 25, 2017
@tkoeppe tkoeppe added this to the C++17 IS milestone Mar 25, 2017
@zygoloid zygoloid merged commit b7964a9 into cplusplus:master Apr 8, 2017
@timsong-cpp timsong-cpp deleted the grammarterm_fixes branch July 2, 2018 04:27
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