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

"explicit constexpr" vs. "constexpr explicit" #2371

Closed
CaseyCarter opened this issue Oct 25, 2018 · 14 comments
Closed

"explicit constexpr" vs. "constexpr explicit" #2371

CaseyCarter opened this issue Oct 25, 2018 · 14 comments

Comments

@CaseyCarter
Copy link
Contributor

We now have both usages in various standard library components.

Which specifier should come first? In years gone by, it made sense for constexpr - the "weird new" specifier - to be first. Now that everything is constexpr, it seems to be that explicit is actually the "weird/uncommon" specifier and it should be uniformly first.

I'll submit a PR for whichever style people prefer, it's more important to me that we be consistent than that we use my preferred style.

@daveedvdv
Copy link
Contributor

I prefer "explicit" to be first. "constexpr" is still semantically the odd one because in some cases (variables) it affects the type (though never when combined with "explicit"). I therefore like to make it the last non-type-specifier before any type specifiers.

@hubert-reinterpretcast
Copy link
Contributor

I also prefer "explicit" to be first; the fact that a function can be called in a constant expression is not as much a part of the interface as whether or not the function would be called implicitly.

@zygoloid
Copy link
Member

Another point: now we have conditional explicit, which is more readable:

constexpr
explicit(some long expression)
MyClass(param1, param2, param3);

... or ...

explicit(some long expression)
constexpr MyClass(param1, param2, param3);

? I prefer the latter. We should also consider what the correct relative order of virtual and constexpr will be when we allow both.

Proposed ordering:

  1. friend / typedef / storage-class-specifier / function-specifier
  2. constexpr
  3. inline
  4. const (const west, const best)
  5. volatile
  6. unsigned / signed
  7. short / long
  8. other type-specifiers

OK?

@daveedvdv
Copy link
Contributor

I'd switch 2 and 3.

(And "Boo" on 4. ;-)

(But since 4 is the state of reality, I think it makes extra sense for "constexpr" to be just before that.)

@zygoloid
Copy link
Member

I think 2 vs 3 only comes up for global variables (everywhere else the constexpr implies the inline), so we're talking

inline constexpr int n = 123;

versus

constexpr inline int n = 123;

I think I agree, the former is better.

@zygoloid
Copy link
Member

I've updated the wiki with the style rule:

https://github.com/cplusplus/draft/wiki/Specification-Style-Guidelines#formatting-declarations-and-definitions

@CaseyCarter Anything else you need to put together the pull request?

@Quuxplusone
Copy link
Contributor

Quuxplusone commented Oct 26, 2018

I prefer the order

static inline constexpr explicit ...

I strongly prefer to see explicit Foo(int) together as a greppable unit. I don't want to see sometimes explicit Foo(int) and other times explicit constexpr Foo(int). Constexprness is not usually significant to me, so it should be relegated far away from the focus of the line (which is Foo). explicit is more important, and also semantically relevant to the name Foo, so it should go right next to Foo.

That said, I can happily use this style in my own codebase regardless of whether the Standard ever agrees with me. 🙂

(EDIT to add: I hadn't thought of virtual. Personally I think I would say static inline constexpr virtual explicit .... As Richard points out, it is possible to have an inline constexpr virtual explicit operator int().)

@zygoloid
Copy link
Member

That's a really good point. Especially for conversion functions, explicit operator T feels much more like a single "unit" of information, and putting a constexpr in the middle seems wrong. (I just checked a large C++ codebase, and there are 50% more inline explicit than explicit inline, which also sways me in that direction.) I also don't think this harms Daveed's "constexpr with the type" desire, because when using explicit you aren't adding a const to the type.

So, I'm happy with the proposed change (move explicit, but not virtual, to after inline and constexpr). I'm still a little concerned that this will make long conditional explicits hard to read, but we actually always use explicit(see below) anyway, so maybe that's not a big deal.

@hubert-reinterpretcast
Copy link
Contributor

I have never had the need to specifically grep for conversion operators that are explicit. Where conversion operators (or constructors) are grouped together (which is good practice), being able to quickly gloss past unconditionally explicit candidates has been useful. Having explicit first helps there. Even with conditionally explicit candidates, being able to easily tell that a group of conditions are the same (or more generally, how they are related) would also be helpful.

@Quuxplusone
Copy link
Contributor

Where conversion operators (or constructors) are grouped together (which is good practice), being able to quickly gloss past unconditionally explicit candidates has been useful

Does this also imply that you would group all explicit constructors together, and then all non-explicit constructors together after them (or perhaps vice versa)? I don't think I do that in general... but then, personally I rarely write any non-explicit constructors. (Except for special members, which get grouped together anyway.)

@FrankHB
Copy link
Contributor

FrankHB commented Oct 26, 2018

First of all, I'd always usestatic inline rather than inline static, to be consistent with ISO C:

WG14 N1570

6.11.5 Storage-class specifiers

1 The placement of a storage-class specifier other than at the beginning of the declaration
specifiers in a declaration is an obsolescent feature.

For semantic similarity reasons, thread-local comes first, and constexpr comes after.

I feel no particular reasons to order other specifiers. But for consistency, I used to separate and order them in 2 groups in my code:

  1. attributes to the whole declaration except in group 2/explicit/friend/ other specifiers/macros to be expand to them (I use no typedef now)

  2. constexpr/inline/__attribute__((visibility(*)))/macro names to be expand to them

The second group is concerned with ODR. (BTW, A minor problem is Clang++ complains about __attribute__((visibility("default"))) [[noreturn]] but not __attribute__((visibility("default"))) __attribute__((noreturn)), while G++ accepts both...)

Once the grouping order is fixed, either order looks good to me, though I now agree inline constexpr is more natural to constexpr inline.

@zygoloid What about #119? (The title is exactly the same, XD)

@zygoloid
Copy link
Member

For semantic similarity reasons, thread-local comes first, and constexpr comes after.

Agreed, but I'd hope this situation never arises in the standard library :)

(BTW, A minor problem is Clang++ complains about __attribute__((visibility("default"))) [[noreturn]] but not __attribute__((visibility("default"))) __attribute__((noreturn)), while G++ accepts both...)

Well, GCC's __attribute__s are decl-specifiers, whereas [[attribute]]s are not; as a general rule, the latter must precede all decl-specifiers (when appertaining to the declarator-id) or follow all decl-specifiers (when appertaining to the type)... but it does make sense to support that so that you can use a sequence of macros expanding to attributes without caring what kind they are.

@zygoloid What about #119? (The title is exactly the same, XD)

I'm coming around to the idea that explicit should bind tightly, partly on the basis that an explicit operator or explicit constructor are really quite different things from implicit conversions. The editorial team will discuss this in-person at San Diego.

@zygoloid zygoloid added the decision-required A decision of the editorial group (or the Project Editor) is required. label Oct 30, 2018
@jensmaurer
Copy link
Member

jensmaurer commented Nov 6, 2018

Editorial meeting: See list by @zygoloid, function-specifier (such as a explicit) comes after constexpr, but virtual is early. See wiki page "guidelines", which was updated.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Nov 6, 2018
CaseyCarter added a commit to CaseyCarter/draft that referenced this issue Nov 17, 2018
CaseyCarter added a commit to CaseyCarter/draft that referenced this issue Nov 18, 2018
zygoloid pushed a commit that referenced this issue Nov 26, 2018
@jensmaurer
Copy link
Member

$ grep "explicit constexpr" *.tex
currently yields no hits, so let's consider this issue fixed.
Feel free to open a new issue with specific pointers to related "guideline" violations.

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

No branches or pull requests

7 participants