<div dir="ltr">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.<div><br></div><div>...but on each individual point, I'm not sure I agree.</div><div><br></div><div>(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.</div><div><br></div><div>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).</div><div><br></div><div>There has to be a better reason. Yeah, I know, that's just a <i>possible</i> reason.</div><div><br></div><div>(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.</div><div><br></div><div>(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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="color:rgb(80,0,80);font-size:12.8px">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.</div></blockquote><div>[...]</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> <span style="color:rgb(80,0,80);font-size:12.8px">A nit: push_back moves from temporaries rather than copying, though for some types the two are the same.</span></blockquote><div><br></div><div>In the way that I (possibly incorrectly) understand move semantics, that actually sounds worse.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">assuming that all of the compilers supported for building are able to handle it.</blockquote><div><br></div><div>...clang builds on non-C++11-compliant compilers? Huh?</div><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br><div class="gmail_quote">On Sun, Jan 17, 2016 at 7:10 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Sun, Jan 17, 2016 at 4:02 PM, James Dennett via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Sun, Jan 17, 2016 at 3:50 PM, <Alexander G. Riccio> via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">There are many places in clang where I see code like this:<div><pre style="font-family:Consolas;color:gainsboro;background:rgb(30,30,30)"><span style="color:rgb(218,218,218)">Checkers</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(200,200,200)">push_back</span><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(78,201,176)">CheckerInfo</span><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(127,127,127)">fn</span><span style="color:rgb(180,180,180)">,</span> <span style="color:rgb(127,127,127)">name</span><span style="color:rgb(180,180,180)">,</span> <span style="color:rgb(127,127,127)">desc</span><span style="color:rgb(180,180,180)">));</span>
</pre></div><div>The equivalent use of emplace_back would be:</div><div><div><pre style="font-family:Consolas;color:gainsboro;background:rgb(30,30,30)"><span style="color:rgb(218,218,218)">Checkers</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(200,200,200)">emplace_back</span><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(127,127,127)">fn</span><span style="color:rgb(180,180,180)">,</span> <span style="color:rgb(127,127,127)">name</span><span style="color:rgb(180,180,180)">,</span> <span style="color:rgb(127,127,127)">desc</span><span style="color:rgb(180,180,180)">);</span></pre></div></div><div><div><div><div><div dir="ltr"><div><span style="font-size:12.8px">....which is (a) more concise, and (b) more efficient. Is there any reason why clang uses push_back instead of emplace_back? </span></div></div></div></div></div></div></div></blockquote><div><br></div></span><div>Some possible reasons:</div><div>(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;</div><div>(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;</div><div>(3) push_back is more conventional and familiar to more developers.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div dir="ltr"><div><span style="font-size:12.8px">Are there any guidelines as to where push_back should be used instead of emplace_back?</span></div></div></div></div></div></div></div></blockquote><div><br></div></span><div>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.</div><div><br></div><div>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.</div></div></div></div></blockquote><div><br></div></span><div>^ this. (& general agreement to the rest)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div dir="ltr"><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">If not, is there any reason to keep using push_back? Why not just replace all uses of push_back with emplace_back?</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">In llvm/tools/clang/lib alone, I count (via grep -r -n -I push_back lib|wc -l) 5,421 uses of push_back. </span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">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?</span><br></div><div dir="ltr"><span style="font-size:12.8px"><br></span></div><div>Background info on <a href="http://en.cppreference.com/w/cpp/container/vector/emplace_back" target="_blank">emplace_back</a> for those who are unfamiliar:</div><div>emplace_back <a href="http://stackoverflow.com/questions/4303513/push-back-vs-emplace-back/4306581#4306581" target="_blank">in-place-constructs (placement new) an element instead of creating a temporary</a> and then copy constructing (push_back does this).</div><div dir="ltr"><span style="font-size:12.8px"><br></span></div></div></div></div></div></div></div></blockquote><div><br></div></span><div>A nit: push_back moves from temporaries rather than copying, though for some types the two are the same.</div><div><br></div><div>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.</div><div><br></div><div>-- James</div><div><br></div></div></div></div>
<br></span>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div>