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

Tue Jan 19 20:06:25 PST 2016

As a whole, I agree with this, and can stick to it. I think that those
rules about emplace_back should be written up somewhere as part of the
general sidelines, even if only as a footnote.

...but on each individual point, I'm not sure I agree.

(1) I think it's intuitive that forcing users to understand exactly what
they're doing is a good idea, but I don't use intuition as a substitute for
evidence.

I'm not convinced that increasing the mental workload (requiring explicit
knowledge of LHS's type) actually improves code quality. It's the same
basic reason that C's raw pointers/lack-of-destructors don't necessarily
improve code quality by telling readers that resources need to be released
(readers can infer that a call to free means that memory is dynamic, but
that's assuming that call is correct as written).

There has to be a better reason. Yeah, I know, that's just a *possible*
reason.

(2) Is a very serious problem. I  didn't know about it, it's bit
surprising, and is a very good reason to not use emplace_back with raw
pointers.

(3) That's a good reason for leaving old code as-is, but not so much for
new code. It's not a huge conceptual difference, so I don't expect a
learning curve. I don't even think it's really a stylistic change... code
using emplace_back looks the same, flows the same way, and (except for #2?)
functions the same way.

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

>  A nit: push_back moves from temporaries rather than copying, though for
> some types the two are the same.

In the way that I (possibly incorrectly) understand move semantics, that
actually sounds worse.

The way that I understand move semantics is that the standard disallows
move elision (i.e. no "as if" rule), and thus push_back's construction &
moving-from a temporary is unnecessary. If exceptions are not in play, then
it's just wasteful.

assuming that all of the compilers supported for building are able to
> handle it.

...clang builds on non-C++11-compliant compilers?  Huh?

Sincerely,
Alexander Riccio
--
"Change the world or go home."
about.me/ariccio

<http://about.me/ariccio>
If left to my own devices, I will build more.
⁂

On Sun, Jan 17, 2016 at 7:10 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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/20160119/09009bb3/attachment.html>