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

Daniel Berlin via cfe-dev cfe-dev at lists.llvm.org
Thu Dec 29 05:54:43 PST 2016


>
>
> From yesterday discussion, Daniel Berlin proposed using emplace_back
> everywhere to make code easier to maintain. I think it is valid argument,
> but I believe it would reduce readability.
>

Just to be clear; I proposed not trying to switch uses back and forth
without real data, and to come to some agreement about what should be
written in the first place, preferably based on real data, and then,
switching in some organized fashion, not just randomly :)

IE Either have a clear coding standard and enforce it, or leave uses alone
one way or the other without some demonstration that it actually matters.

I would *personally* prefer to just use emplace_back everywhere, and not
try to switch between the two, without demonstration of real benefit.

(i'd also accept the other way, use push_back everywhere, and not try to
switch between the two, without demonstration of real benefit).

This preference is based on a simple truth: People suck at deciding when to
use one or the other. Yes, i know how to use it and when to use it.  Lots
of our types are cheaply movable, etc, so you probably won't see any
performance difference even when using them "right".  People have enough
style/etc issues without having to try to think hard about this.

So that isn't what I would necessarily propose for LLVM.

For LLVM, my answer would be "either make tool do what we want, force use
of tool, or be consistent and obvious with what we want and then fix it if
it leads to performance issue"




> There is also other reason - emplace_back can't take arguments of some
> types like bitfields, NULL, static member. Using emplace can also lead to
> memory leak in case of smart ptrs
> "
>
> auto *ptr = new int(1);v.push_back(std::unique_ptr<int>(ptr));
>
> This is because replacing it with emplace_back could cause a leak of this
> pointer if emplace_back would throw exception before emplacement (e.g.
> not enough memory to add new element).".
>

This seems, IMHO, not a useful argument for LLVM since it does not try to
use exception based error handling to recover.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20161229/5a78799e/attachment.html>


More information about the cfe-dev mailing list