<div dir="ltr">Somewhat unfortunately, we have two discussions here:<div><br></div><div>- Clang-tidy has checks that might improve code -- should we deploy them? If so which?</div><div><br></div><div>I'll address this in a separate email, and focus this one on:</div><div><br></div><div>- Should we have coding standards around 'push_back' and 'emplace_back', and if so, what should they say?</div><div><br></div><div>I think the amount of confusion makes a coding standard useful.</div><div><br></div><div>As for what it should say, I tend to agree with Dave here. In particular:</div><div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 29, 2016 at 12:03 PM David Blaikie via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Thu, Dec 29, 2016 at 11:46 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Dec 29, 2016, at 11:20 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_328146586727142681m_-6648727749751248484Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Thu, Dec 29, 2016 at 10:04 AM Mehdi Amini via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="gmail_msg" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" 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 class="gmail_msg" style="word-wrap:break-word"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Dec 29, 2016, at 5:54 AM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" class="gmail_msg" target="_blank">dberlin@dberlin.org</a>> wrote:</div><br class="gmail_msg m_328146586727142681m_-6648727749751248484m_1399359644854761192Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" 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 dir="ltr" class="gmail_msg"><br class="gmail_msg"><div class="gmail_msg">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.</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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 :)</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I would *personally* prefer to just use emplace_back everywhere, and not try to switch between the two, without demonstration of real benefit.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">(i'd also accept the other way, use push_back everywhere, and not try to switch between the two, without demonstration of real benefit).</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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.</div></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg" style="word-wrap:break-word"><div class="gmail_msg"><div class="gmail_msg">I agree that “people suck at deciding”, me in the first place in the line, and that’s why I like clear and good guideline and stick to it unless "demonstration of real benefit”.</div><div class="gmail_msg">I also think we can have clear guideline that are different from “always use emplace_back” or “always use push_back”, like Piotr is suggesting.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I don't think Piotr is suggesting "always use push_back" but "always use push_back when it's valid" & I'd second this.<br class="gmail_msg"></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Define “valid”? Just that it will compile?</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">Yep</div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">To show a simpler example:<br class="gmail_msg"><br class="gmail_msg">std::unique_ptr<T> u(v);<br class="gmail_msg"><br class="gmail_msg">std::unique_ptr<T> u = v;<br class="gmail_msg"><br class="gmail_msg">I'd generally advocate for using copy init (the second form - it doesn't copy in this case, it moves) over direct init (the first form) because it's more legible - copy init can only execute implicit ctor operations, whereas direct init can invoke implicit and explicit operations. So from a readability perspective seeing the latter is easier than seing the former - I know the operation is further constrained to simpler/less interesting operations (eg: 'v' can't be a raw pointer in the second form, it might be (& then I have to worry about whether that's an ownership transfer that's intended), etc)<br class="gmail_msg"><br class="gmail_msg">& push_back V emplace_back is the same choice - push_back means only implicit ops are used and so it's more readable</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">I don’t see what’s more readable about “only implicit ctor”.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">It limits the set of operations that can be performed by the code. So when I read it there's less I have to think about/consider. (& specifically, the implicit operations tend to be simpler/safer/more obvious - copy/move or operations that are similar - not complex/interesting things like "taking ownership from a raw pointer" or "creating a vector of N elements")</div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"> Emplace is very explicit that we intended to construct an object, I don’t understand what hurts readability here?</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">Going back to the example above, given the following two lines:<br class="gmail_msg"><br class="m_328146586727142681inbox-inbox-Apple-interchange-newline gmail_msg">std::unique_ptr<T> u(foo());<br class="gmail_msg">std::unique_ptr<T> u = foo();<br class="gmail_msg"><br class="gmail_msg">(& the equivalent: emplace_back(foo()) V push_back(foo()) for a vector of unique_ptr)<br class="gmail_msg"><br class="gmail_msg">the copy init/push_back are simpler to read because they aren't as powerful - I don't have to wonder if something is taking ownership there (or if I'm creating a vector of N ints, etc). I know it's a simple/obvious operation, generally (because others shouldn't be implicit).</div></div></div></blockquote><div><br></div><div>This is exactly where I come down as well.</div><div><br></div><div>Another useful way to think about it is "what do I need to understand to understand the semantics of this operation".</div><div><br></div><div>In order to understand push_back, I need only read its documentation. The type going in will have to have value semantics (potentially with moves). Otherwise it will be an error. And push_back's documentation says what it does.</div><div><br></div><div>In order to understand a given emplace_back call I have to *both* read its documentation and the documentation for all of the constructors on the type.</div><div><br></div><div><br></div><div>Still another way to see the consequence of this is to look at the nature of compiler errors when a programmer makes a mistake.</div><div><br></div><div>With emplace_back, if you fail to call the constructor correctly, all of the error messages come with a layer (or many layers) of template instantiation. With push_back(T(...)), the constructor call is direct and the error messages are as well.</div><div><br></div><div>With emplace_back, if you are converting from one type to another and the conversion fails, you again get template backtraces. With push_back, all the conversions happen before the push_back method and so the error is local to your code.</div></div></div></div>