llvm dev - Jan 2017 - [cfe-dev] Modernizing LLVM Coding Style Guide and enforcing Clang-tidy

> On Jan 9, 2017, at 12:47 PM, Renato Golin <renato.golin at
linaro.org> wrote:
> 
> On 9 January 2017 at 19:04, Mehdi Amini <mehdi.amini at apple.com>
wrote:
>> 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.

— 
Mehdi

> 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

On 9 January 2017 at 20:48, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> Either one of us is mistaken, but I find yourself being fairly confident
here…
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

On Mon, Jan 9, 2017 at 12:48 PM, Mehdi Amini via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
> > On Jan 9, 2017, at 12:47 PM, Renato Golin <renato.golin at
linaro.org>
> wrote:
> >
> > On 9 January 2017 at 19:04, Mehdi Amini <mehdi.amini at
apple.com> wrote:
> >> 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

>
> —
> Mehdi
>
>
> > 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
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170110/ea438ea0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: download (8).png
Type: image/png
Size: 329204 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170110/ea438ea0/attachment-0001.png>

> On Jan 10, 2017, at 12:33 AM, Sean Silva <chisophugis at gmail.com>
wrote:
> 
> 
> 
> On Mon, Jan 9, 2017 at 12:48 PM, Mehdi Amini via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> 
> > On Jan 9, 2017, at 12:47 PM, Renato Golin <renato.golin at
linaro.org <mailto:renato.golin at linaro.org>> wrote:
> >
> > On 9 January 2017 at 19:04, Mehdi Amini <mehdi.amini at apple.com
<mailto:mehdi.amini at apple.com>> wrote:
> >> 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
<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.
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.

— 
Mehdi


> The real coding standard is the one that a critical mass of LLVM developers
will comment on when they find something objectionable.
> 
> <download (8).png>
> ​
> -- Sean Silva
>  
> 
> —
> Mehdi
> 
> 
> > 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
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170110/55d63abc/attachment.html>