[cfe-dev] Guidelines for use of emplace_back vs push_back in clang?

[James Dennett via cfe-dev]

On Sun, Jan 17, 2016 at 3:50 PM, <Alexander G. Riccio> via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> There are many places in clang where I see code like this:
>
> Checkers.push_back(CheckerInfo(fn, name, desc));
>
> The equivalent use of emplace_back would be:
>
> Checkers.emplace_back(fn, name, desc);
>
> ....which is (a) more concise, and (b) more efficient. Is there any reason
> why clang uses push_back instead of emplace_back?
>

Some  possible reasons:
(1) push_back often tells readers the type; with emplace* they have to know
the type of the lhs to see what's being constructed;
(2) emplace_back will silently invoke `explicit` constructors, making it
possible to accidentally emplace_back a raw pointer into a container of
smart pointers, possibly leading to a double-delete when the container
takes ownership;
(3) push_back is more conventional and familiar to more developers.


> Are there any guidelines as to where push_back should be used instead of
> emplace_back?
>

If you already have an object of the type you've planning to append,
there's no benefit to emplace_back -- it's just an obfuscated way to
request moving/copying, which is what push_back does more obviously and
safely.

Personally I reserve emplace_back for times when either it's required by
language rules (e.g, a type isn't movable) or it's required for efficiency,
and stick to the simpler/safer choice the rest of the time.


>
> If not, is there any reason to keep using push_back? Why not just replace
> all uses of push_back with emplace_back?
>
> In llvm/tools/clang/lib alone, I count (via grep -r -n -I push_back lib|wc
> -l) 5,421 uses of push_back.
>
> Not all of those are for STL containers (not many of the llvm containers
> have emplace_back), but surely this is some low-hanging (performance)
> fruit? If not in release builds (where push_back's copy constructor call is
> likely elided), then in debug builds?
>
> Background info on emplace_back
> <http://en.cppreference.com/w/cpp/container/vector/emplace_back> for
> those who are unfamiliar:
> emplace_back in-place-constructs (placement new) an element instead of
> creating a temporary
> <http://stackoverflow.com/questions/4303513/push-back-vs-emplace-back/4306581#4306581>
> and then copy constructing (push_back does this).
>
>
A nit: push_back moves from temporaries rather than copying, though for
some types the two are the same.

There might be merit in updating for Clang/LLVM code to use emplace_back in
some cases, assuming that all of the compilers supported for building are
able to handle it.  I don't have a strong opinion, though I do caution
against overuse of emplace* functions.

-- James
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160117/07568c59/attachment.html>