The last two are enforced by compiler warnings now. The second is hard
because of bitfields.
I object to the first. If you need a new type name, use a typedef. It's
time honored and everyone, including C programmers, will know what you're
doing. I don't understand why people push the new thing just for the sake
of new-ness.
Are there any other comments about changing style guide?
I would like to add points like- prefer "using' instead of "typedef"
- use default member initialization
struct A {
void *ptr = nullptr;
};(instead of doing it in constructor)
- use default, override, delete
- skip "virtual" with overrideThe last point is to get to consensus with
push_back({first, second})
or
emplace_back(first ,second);It might be a bit noisy, but I'd be inclined to start a separate thread
for each of these on llvm-dev with a clear subject line relevant to each
one. So they don't get lost and some of them don't get drowned out by the
discussion of others, etc.Sure, I can do it, but at least now I don't see much of attention of
people to any of the subject, so I would not like to start 5 empty threads.
I guess as long as the thread is not getting noisy I will keep only one.
Does it sound ok?Perhaps - if no one else pipes up *shrug*
If that's the case, I'd at least suggest submitting the changes to the
style guide separately with clear subject/titles so people reading commits
might see/notice/discuss there.Good idea.
FWIW: I'm also in favor of push_back where valid. Though I'm not sure it's
so much a matter of votes, but justification, etc. As for the others -
sure, they all sound good to me.I totally agree with all the points Chandler pointed out except the 2.b
(multiple or no arugments). I just prefer emplace_back, but I find the
push_back({}) also clean. So I guess small voting would be a good way to
figure out what people prefer here.
Also - once these are in the style guide there's still a separate
discussion to be had about whether automated cleanup is worthwhile & how
best to go about that sort of thing.
That's right, I am not proposing any automatic cleanups (yet).
using handles strictly more cases than typedef, in particular partial specialization of templates. So because we’ll end up with using anyway, uniformity can be valuable. So that could be a motivation: since using is needed anyway, might be better to just use it always (I’m not saying it is a “strong” motivation though, just some piece of rational).
(I also find that typedef of function pointers in particular are terrible, and using is much better for these, but that can be a matter of taste).
The text read "prefer", which I think it's fair. I'm opposed to any
hard rule for the sake of new-ness, consistency, or personal
preference.
If we encode all nit-rules as "prefer", then it should be more like a
guideline than a rule book, which has been our motto for everything
else.
cheers,
--renato
+1 Exactly this.
I don't think C programmer will not understand using. The "=" makes it much
simpler to read, even if it is the first time you see it, which is not the
case of typedef.
typedef MyType::NestedType (*fptr)(const MyOhterType&);
or
using fptr = MyType::NestedType (*)(const MyOhterType&);
Typedefs with function pointers are used in couple of places in LLVM and I
find it terible to read.
So it is not about new-ness. Trust me, I would never use typedef if using
was first 
Clang is smart enough to propose using override, but only if it see that it
was used somewhere, so it is not totally enforced. Also I don't think it
warns about redundant "virtual" if one writes "virtual void foo() override".
Yes, the bitfields is the only one reason we would need initializations in
constructors in some places, but it is rare (as far as what clang-tidy
check have found)
That's right, everyone has some preferences, but as long as most (over 75%)
of the community prefer one thing over another then I guess it would make
sense to convice the rest and make it official by writing it down. Then we
won't have to spend time dicussing the same things over and over on
different reviews. It is just much simpler to point new (or not)
contributor to a rule in style guide.
Piotr
This is not correct according to the number of “should” and the imperative tone for many aspects of http://llvm.org/docs/CodingStandards.html#source-code-formatting
You mistake the tone of the documentation. There are things that
cannot be (exceptions, RTTI), things that are important to get right
(includes vs. forward declaration), things that are preferred
(c++11-isms) and things that are optional and very much depends on the
situation. The four items in the list I replied to fall into the
latter category.
The tone used for each type is appropriate to its enforcement. If you
add compiler errors or warnings, it's pretty easy to enforce.
Everything else will have varying degrees of success, and being
obnoxious about it has never been, and I hope never will be, our way.
We don't force people to run clang-format on patches, we ask when it's
ugly and people do because they believe it's a good thing. When the
formatting doesn't hurt my eyes, I don't ask for clang-format. I
certainly won't start asking people to run clang-tidy, though I'd be
happy if they did. That's personal and with the volume of commits we
have, that last thing we need is people blocking or reverting patches
because they didn't conform to personal preferences, even if they were
encoded in the coding standards.
I also strongly oppose to encoding personal preferences with a
stronger wording that it's warranted. Personal is personal. If it's
legal C++ and it's an appropriate use of the language for the case at
hand, than it's fine. I couldn't care less if you use "using" or
"typedef". I can understand both. "Prefer using" is an interesting
proposition, but refuse patches because they have "typedefs" is silly.
Honestly, my "coding standards" would be as simple as "do whatever
Scott Meyers says you should", but the LLVM one is nice, too. Unless
it's used as a weapon.
cheers,
--renato
Either one of us is mistaken, but I find yourself being fairly confident here…
Try going above the 80 cols and defend it as your personal preference in a review, and let me know how it went.
I'm only speaking from what I've seen happening in the past few years
committing and reviewing code. Our personal opinions don't change what
happened before, though they may try to change the future.
I just hope we don't become nit-pick nazis. I certainly won't become one.
cheers,
--renato
I would prefer using typedefs at least for function pointers. I find either of
Hi,
Sorry I fat fingered an earlier send in the previous email. I was
trying to say:
+1 Exactly this.
I don't think C programmer will not understand using. The "=" makes it much
simpler to read, even if it is the first time you see it, which is not the
case of typedef.typedef MyType::NestedType (*fptr)(const MyOhterType&);
or
using fptr = MyType::NestedType (*)(const MyOhterType&);
I would prefer to please keep using typedefs at least for function
pointers. I find either of
typedef MyType::NestedType (*fptr)(const MyOhterType&);
or
typedef int fptr(const int&);
void f(fptr* ptr) {
...
}
easier to read than the "using" declaration (especially the second
form, with the explicit `fptr* ptr`).
-- Sanjoy
Hi,
Sorry I fat fingered an earlier send in the previous email. I was
trying to say:+1 Exactly this.
I don’t think C programmer will not understand using. The “=” makes it much
simpler to read, even if it is the first time you see it, which is not the
case of typedef.typedef MyType::NestedType (fptr)(const MyOhterType&);
or
using fptr = MyType::NestedType ()(const MyOhterType&);I would prefer to please keep using typedefs at least for function
pointers. I find either oftypedef MyType::NestedType (*fptr)(const MyOhterType&);
or
typedef int fptr(const int&);
void f(fptr* ptr) {
…
}easier to read than the “using” declaration (especially the second
form, with the explicitfptr* ptr).
Not sure I follow. You’re saying this:
typedef int func_type(const int&);
is easier for you to read than this:
using func_type = int(const int&);
?
Hi,
Sorry I fat fingered an earlier send in the previous email. I was
trying to say:>> +1 Exactly this.
>> I don't think C programmer will not understand using. The "=" makes it
much
>> simpler to read, even if it is the first time you see it, which is not
the
>> case of typedef.
>>
>> typedef MyType::NestedType (*fptr)(const MyOhterType&);
>> or
>> using fptr = MyType::NestedType (*)(const MyOhterType&);
>I would prefer to please keep using typedefs at least for function
pointers. I find either oftypedef MyType::NestedType (*fptr)(const MyOhterType&);
or
typedef int fptr(const int&);
void f(fptr* ptr) {
...
}easier to read than the "using" declaration (especially the second
form, with the explicit `fptr* ptr`).Not sure I follow. You're saying this:
typedef int func_type(const int&);
is easier for you to read than this:
using func_type = int(const int&);
I'd say so, one looks like the functions i right. The other looks strange 
Fair enough
(though mostly (going by a quick grep) we end up with (a very rough guess, looking at a grep over llvm - maybe 20 times more common than function typedefs):
typedef void (*fptr)(const int&);
which is a bit more arcane & perhaps easier as:
using fptr = void (*)(const int&);
Then you don’t have to try to remember where the variable name goes in these weird declarations - it’s still pretty ugly, admittedly)
>
>> This is not correct according to the number of “should” and the
imperative tone for many aspects of http://llvm.org/docs/
CodingStandards.html#source-code-formatting
>
> You mistake the tone of the documentation.Either one of us is mistaken, but I find yourself being fairly confident
here…Try going above the 80 cols and defend it as your personal preference in a
review, and let me know how it went.
I doubt most reviewers will notice if you go slightly over 80 cols without
some sort of automated check warning about it. W.r.t. the higher-level
semantic guidelines, no reviewer keeps them all in their head. Just writing
down a rule doesn't buy anything no matter how you write it down. The real
coding standard is the one that a critical mass of LLVM developers will
comment on when they find something objectionable.
-- Sean Silva
Well I believe it still buys you an interesting property: no bike shedding over “personal preference” in any review. There’s a guideline to point at and we can’t instead focus on the important bits.
Hi,
Sorry I fat fingered an earlier send in the previous email. I was
trying to say:>> +1 Exactly this.
>> I don't think C programmer will not understand using. The "=" makes it
much
>> simpler to read, even if it is the first time you see it, which is not
the
>> case of typedef.
>>
>> typedef MyType::NestedType (*fptr)(const MyOhterType&);
>> or
>> using fptr = MyType::NestedType (*)(const MyOhterType&);
>I would prefer to please keep using typedefs at least for function
pointers. I find either oftypedef MyType::NestedType (*fptr)(const MyOhterType&);
or
typedef int fptr(const int&);
void f(fptr* ptr) {
...
}easier to read than the "using" declaration (especially the second
form, with the explicit `fptr* ptr`).Not sure I follow. You're saying this:
typedef int func_type(const int&);
is easier for you to read than this:
using func_type = int(const int&);
?
I never saw syntax
typedef int funct_type(const int&);
I tried to use it and I got compile error for code:
int z(const int&);
int main() {
typedef int fptr(const int&);
fptr f = z;
f(42);
}
where typedef int (*fptr)(const int&)
works.
You can't have that certainty with guidelines.
These 4 examples are less enforceable than 80-columns or
no-tabs-2-spaces rules, despite both sets being personal. They're
clearly guidelines.
All writing it down will buy you is "prefer this, not that", but:
1. If you have a good reason to do that, please do.
2. If the surrounding code uses that pattern, and this is not a
structural change, please repeat.
3. If it makes no difference as to the clarity of the code, who cares?
The most important thing here is the reaction of the community.
Reverting patches because they don't conform to guidelines is a big
mistake. Even 80 columns violation should be fixed later, in place, by
the original committer, after post-commit review.
Blocking patches because they don't conform to personal preferences,
even if they're encoded in the coding standard as "prefer" or
"should", ignoring the other facts I mention above, is also really bad
form.
First, we don't have code stewards to clean up other people's mistakes
or preferences, not we should. This would create a culture of garbage
code, where people don't care about what they commit.
Second, we want developers to feel comfortable contributing to our
project. People DO have different cultures, preferences, levels of
comfort with the languages and the project. But they also have their
own lives and work, and can't spend months polishing small patches,
just because they're not perfect in the view of random developers in
our community. That would be extremely counter-productive and would
ultimately kill the project.
Similar behaviour has killed other OSS projects in the past, so this
is not theoretical.
All that said, I think those 4 *guidelines* are nice to have on our
coding standards. But I'm ultimately against forcing people to run
clang-tidy or blocking/reverting patches because they don't match
perfectly with the coding standards.
cheers,
--renato