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

[Piotr Padlewski via llvm-dev]

Thanks for very accurate responses.
- I totally agree what Dave and Chandler said about explicit and implicit
operations, this is what I meant my first email.
  I believe there are places like
    v.emplace_back(A, B);
  istead of
    v.push_back(make_pair(A, B));b
  That can make code simpler. I think in cases like this we can leave it
for judgement of contributor.
  Having said that I think the case Chandler and Dave mentioned should be
in LLVM style guide.
  @Daniel does this argument seem good enough to prefer push_back instead
of emplace_back?

- About clang-tidy config (Dave):
  We already have clang-tidy config in LLVM and clang - check ".clang-tidy"
file. I feel tho that people don't use clang-tidy as often as clang-format.
  This might be because clang-tidy didn't have enough features that they
would like. I believe that clang-tidy is doing much better and could really
save a lot of time for reviewers and contributors.
  But to make it happen we either have to change LLVM style guide, so we
won't need to discuss it over and over in reviews, or gain clang-format
trust - it is so good people don't question it.

- About enforcing (Sean)
  Sorry, I used wrong word here. I meant to use it the same way as
clang-format, to trust people they send cleaned code by clang-tidy the same
way we trust them that they used clang-format.
  Reviewer can then ask contributor to run clang-tidy instead of listing
things that contributor should change. We are a little far from having
mandatory running of clang-format and clang-tidy on every patch
automatically.

- About workflow (Dave)
  If I remember correctly there is some integration for clang-tidy for vim
(YCM?). There is also clang-tidy-diff, but maybe we should have
git-clang-tidy to make it even simpler.


2016-12-29 23:19 GMT+01:00 Chandler Carruth via cfe-dev <
cfe-dev at lists.llvm.org>:

> Dave pointed out that I didn't complete one aspect of my argument on the
> push_back vs. emplace_back:
>
> On Thu, Dec 29, 2016 at 2:04 PM Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> Still another way to see the consequence of this is to look at the nature
>> of compiler errors when a programmer makes a mistake.
>>
>> With emplace_back, if you fail to call the constructor correctly, all of
>> the error messages come with a layer (or many layers) of template
>> instantiation. With push_back(T(...)), the constructor call is direct and
>> the error messages are as well.
>>
>> With emplace_back, if you are converting from one type to another and the
>> conversion fails, you again get template backtraces. With push_back, all
>> the conversions happen before the push_back method and so the error is
>> local to your code.
>>
>
> This in turn makes me prefer push_back(T(...)) over empalce_back(...). I
> *like* the constructor being called explicitly in the users code if is an
> *explicit* constructor. For things that should be implicit, they already
> are with push_back. If the API of the type went out of its way to make a
> constructor call explicit, I'd like to see the user code actually calling
> it.
>
>
> The consequence of this is very simple guidelines for programmers to:
> don't use emplace_back unless you *must* use it. So for containers of
> non-copyable and non-movable types, it can be very useful and important.
> But in every other case I would rather see push_back (and if an explicit
> constructor call is necessary, an explicit constructor call).
>
> My two cents.
> -Chandler
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161230/c4d66650/attachment.html>