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

CWG Poll 16: P2002R1 Defaulted comparison specification cleanups #3767

Merged
merged 1 commit into from Feb 26, 2020

Conversation

burblebee
Copy link
Contributor

@burblebee burblebee commented Feb 20, 2020

[class.compare.default] Added "was" to "no overload resolution performed".

Fixes #3700.
Fixes cplusplus/papers#739.

Notes/Issues:

  • [everywhere] Please check my \defns and \indextexts - I wasn't sure how I should specify them.
  • [class.compare.default] We just say "operator@" not "operator@ function". We're also inconsistent in whether we encode is as "operator \tcode{@}" v. "\tcode{operator@}".
  • [class.compare.default] We usually say "defaulted comparison operator function" instead of "defaulted comparison function".
  • [class.compare.default] In the last 2 bullets of the new paragraph after p2, the "for" at the end of each bullet is hard to parse correctly: "
    ** for an operator<=> whose return type is auto or for an operator==, for a comparison between an element of the expanded list of subobjects and itself, or
    ** for a secondary comparison operator @, for the expression x @ y."

@jensmaurer jensmaurer added this to the post-2020-02 milestone Feb 20, 2020
@jensmaurer jensmaurer changed the title P2002R1 Defaulted comparison specification cleanups CWG Poll 16: P2002R1 Defaulted comparison specification cleanups Feb 21, 2020
source/classes.tex Outdated Show resolved Hide resolved
A defaulted comparison operator function for class \tcode{C}
A defaulted comparison operator function for class \tcode{C} that
is not defined as deleted is
\indextext{operator!comparison!defaulted!implicitly defined}%
Copy link
Member

Choose a reason for hiding this comment

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

Please check the existing index entries and find a suitable place. This one isn't.
Maybe "operator function!comparison!implicitly-defined".

Copy link
Contributor Author

@burblebee burblebee Feb 25, 2020

Choose a reason for hiding this comment

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

For all my index entries, I do scan the existing ones and to try and find a suitable place. As I said in my "Notes/issues", it isn't clear how we should write index entries. I don't like having so many levels either, but which part to leave out, and which parts to combine?

That said, I see we already have existing definitions for "implicitly defined", none of which are top-level, and appear to use "implicitly defined" and "implicitly-defined" interchangeably throughout the wording. In [class.copy.ctor]/p12 for example, we define "implicitly defined" under copy/move constructors, then use "implicitly-defined" in the very same paragraph. I would like to a see top-level entry for "implicitly defined" which refers to these other cases in a second level, and to clean up the confusion between "implicitly defined" and "implicitly-defined" .

For this case, we don't have "operator function" but we do have "operator", so I'll just remove the "defaulted" level and remove the top level definition to be consistent with the other definitions. Thanks!


\pnum
A binary operator expression \tcode{a @ b} is
\indextext{operator!\idxcode{@}!usable}%
Copy link
Member

Choose a reason for hiding this comment

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

drop the \idxcode{@}; it doesn't help in the index entry.

Copy link
Contributor Author

@burblebee burblebee Feb 25, 2020

Choose a reason for hiding this comment

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

This is how we format operators in the index, e.g.:

\indextext{operator!\idxcode{+=}}%
\indextext{\idxcode{++}|see{operator, increment}}%
\indextext{\idxcode{:}!label specifier}%
\indextext{\idxcode{for}!scope of declaration in}%

That said, the indexes for operators and keywords also needs a cleaned up. E.g. we have \indextext{\idxcode{for}!scope of declaration in}% but no top-level for, just \indextext{statement!\idxcode{for}}%.

source/classes.tex Outdated Show resolved Hide resolved
source/classes.tex Outdated Show resolved Hide resolved
source/classes.tex Outdated Show resolved Hide resolved
A defaulted relational operator function\iref{over.binary}
for some operator \tcode{@}
\indextext{operator!comparison!secondary}%
A \defn{secondary comparison operator} is
Copy link
Member

Choose a reason for hiding this comment

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

drop the \indextext and just say \defnadj{secondary}{comparison operator}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with \defnadj{secondary}{operator!comparison} since we don't have a top level index entry for "comparison operator".

Copy link
Member

Choose a reason for hiding this comment

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

I don't think "!" renders correctly in the \defnadj. You need to do a manual \defnx if you want to get clever.

@jensmaurer jensmaurer added the changes requested Changes to the wording or approach have been requested and not yet applied. label Feb 25, 2020
@burblebee burblebee removed the changes requested Changes to the wording or approach have been requested and not yet applied. label Feb 25, 2020
burblebee pushed a commit that referenced this pull request Feb 25, 2020
@zygoloid zygoloid force-pushed the motions-2020-02-cwg-16 branch 3 times, most recently from 240ee24 to c80f23a Compare February 26, 2020 02:32
[dcl.fct.def.default] Wording massaged to resolve minor merge conflict with P1779R3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants