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

[lib] LWG 3239: inline definition in the header / class synopsis #3110

Closed
jensmaurer opened this issue Jul 31, 2019 · 13 comments
Closed

[lib] LWG 3239: inline definition in the header / class synopsis #3110

jensmaurer opened this issue Jul 31, 2019 · 13 comments
Labels
lwg Issue must be reviewed by LWG.

Comments

@jensmaurer
Copy link
Member

jensmaurer commented Jul 31, 2019

Beyond the accepted =default and =delete, actual code definitions (with braces) have crept up in the header or class synopses with the recent batch of motions. The alternative presentation would be to have an itemdecl / itemdescr with "Effects: Equivalent to" codeblock.

For example, this applies to constructors and inline friends.

We should clarify the style guidelines to say what we want here.

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Jul 31, 2019
@jwakely
Copy link
Member

jwakely commented Jul 31, 2019

For hidden friends this is intentional, to show that they are supposed to be defined inline and not have a namespace-scope declaration. Simply defining them inline isn't sufficient for that though (nothing prevents an implementation from adding a declaration outside the class), so there's a proposal to define new wording that states the intent clearly.

For member functions there's a sort-of-policy to define then inline if they use return type deduction, which is @AlisdairM's idea and is proving unpopular.

@jwakely
Copy link
Member

jwakely commented Jul 31, 2019

For constructors we've been defining them inline for some time, when the constructor delegates to another and has an empty body.

@jensmaurer
Copy link
Member Author

I've started a description here:
https://github.com/cplusplus/draft/wiki/Specification-Style-Guidelines#inline-definitions-in-synopses

I'm mostly ok (although not enthusiastic) regarding the rule for the delegating constructors, but the hidden friends are not helpful as-is. We should rather have blanket wording that all friends shown in a class synopsis shall not have namespace-level declarations (unless expressly shown otherwise), which automatically implies that the friend declaration must also be the definition (but that's an implementation detail).

@jwakely
Copy link
Member

jwakely commented Jul 31, 2019

the hidden friends are not helpful as-is

That's what http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1601r0.pdf aims to fix.

@jensmaurer
Copy link
Member Author

jensmaurer commented Jul 31, 2019

Except it doesn't add wording that prevents an implementation from adding a namespace-scope redeclaration.

@jwakely
Copy link
Member

jwakely commented Jul 31, 2019

A namespace-scope redeclaration would make this requirement hard to meet:

  1. In addition, the following sentence should become part of the entity’s specification via an
    accompanying Remarks: element: “This function is to be found via argument-dependent
    lookup only.”4

That suggests that every hidden friend does need a separate \itemdecl where the Remarks: element will appear, as the footnote says:

4 [One] might envision that a future draft of the C++ standard would include blanket library wording that may make such per-function specification unnecessary. Until then, we recommend that each befriended function be accompanied by the above-recommended wording restricting successful lookup of befriended functions to ADL only.

@jwakely
Copy link
Member

jwakely commented Jul 31, 2019

Or we could just add that blanket wording now, saying that all friend functions defined inline in the class synopsis are to be found via ADL only (maybe with a note saying that means no namespace-scope declaration). That would be a much smaller edit and would fix all existing uses of the idiom.

@jensmaurer
Copy link
Member Author

So, the result seems to be:

  • Specify in the library front matter that friend functions defined in the class definition are found via ADL only.
  • As a library editorial policy, use { \seebelow } in the class synopsis if the definition of the friend function gets large.
  • To be decided: Do we want to treat all inline friends like that, so that we have all/most of the description of library functions with \itemdecl / \itemdescr instead of having non-trivial code in the class synopsis.

@CaseyCarter
Copy link
Contributor

CaseyCarter commented Aug 5, 2019

Specify in the library front matter that friend functions defined in the class definition are found via ADL only.

Is there any reason to restrict this to only those friend functions that are defined in the class definition, as opposed to all friend functions declared in a class definition and not redeclared outside that class definition at namespace scope? To my knowledge we have no non-hidden friends declared in the Library since we largely leave it unspecified how non-member functions obtain access to class members needed to achieve their specification. Conversely, we have seen a couple examples of functions that couldn't be inlined into a class definition due to the need to specify some non-code behavior; it would be a shame if we eventually stumble across such a function that we'd like to be a hidden friend.

EDIT: Bah, the second bullet above addresses my problem scenario. Let's ignore that bit of rationale, and revert my statement to "it would be nice to keep the location of function specification (inline in the class definition vs. \itemdecl / \itemdescr) orthogonal to that function's hidden-friendliness and/or deduced return type-ness unless there's a specific reason to constrain ourselves to only friend functions defined directly in class definitions.

@jensmaurer jensmaurer changed the title [lib] inline definition in the header / class synopsis [lib] LWG 3239: inline definition in the header / class synopsis Aug 5, 2019
@jensmaurer
Copy link
Member Author

jensmaurer commented Aug 5, 2019

@CaseyCarter, I agree. In general, we should keep the amount of inline code to a minimum, and favor \itemdecl / \itemdescr instead. However, the \itemdecl itself might be construed as an out-of-line declaration, with the resulting detrimental effect on an intended "ADL-only" function. So, we need clear front-matter wording that any friend function shown in a class synopsis is of the "ADL-only" variety.

This needs LWG coordination via LWG issue 3239.

@jensmaurer jensmaurer added the lwg Issue must be reviewed by LWG. label Aug 5, 2019
@jwakely
Copy link
Member

jwakely commented Aug 6, 2019

However, the \itemdecl itself might be construed as an out-of-line declaration

Right. Technically, only a namespace-scope declaration in a synopsis codeblock counts as such, but we should avoid that subtlety.

@jensmaurer jensmaurer added decision-required A decision of the editorial group (or the Project Editor) is required. and removed decision-required A decision of the editorial group (or the Project Editor) is required. labels Sep 10, 2019
@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Sep 30, 2019
@jensmaurer
Copy link
Member Author

Editorial teleconference: Let's wait until the LWG issue is resolved.

@jensmaurer
Copy link
Member Author

P1965R0 got merged, and with the updated editorial guidance, this seems to be moot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Development

No branches or pull requests

3 participants