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

[David Blaikie via cfe-dev]

On Sun, Jan 17, 2016 at 4:02 PM, James Dennett via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> 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.
>

^ this. (& general agreement to the rest)


>
>
>>
>> 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
>
>
> _______________________________________________
> 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/cfe-dev/attachments/20160117/e82eba84/attachment.html>