<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 9, 2017 at 6:17 AM Piotr Padlewski 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">Are there any other comments about changing style guide?<div class="gmail_msg">I would like to add points like</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">- prefer "using' instead of "typedef"</div><div class="gmail_msg">- use default member initialization</div><div class="gmail_msg">struct A {</div><div class="gmail_msg"> void *ptr = nullptr;</div><div class="gmail_msg">};</div></div></blockquote><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_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">(instead of doing it in constructor)</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">- use default, override, delete</div><div class="gmail_msg">- skip "virtual" with override</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">The last point is to get to consensus with </div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">push_back({first, second}) </div><div class="gmail_msg">or</div><div class="gmail_msg">emplace_back(first ,second);</div></div></blockquote><div><br><br class="inbox-inbox-Apple-interchange-newline">It might be a bit noisy, but I'd be inclined to start a separate thread for each of these on llvm-dev with a clear subject line relevant to each one. So they don't get lost and some of them don't get drowned out by the discussion of others, etc.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg">2016-12-30 12:26 GMT+01:00 Piotr Padlewski <span dir="ltr" class="gmail_msg"><<a href="mailto:piotr.padlewski@gmail.com" class="gmail_msg" target="_blank">piotr.padlewski@gmail.com</a>></span>:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="gmail_msg">2016-12-30 11:34 GMT+01:00 Chandler Carruth <span dir="ltr" class="gmail_msg"><<a href="mailto:chandlerc@gmail.com" class="gmail_msg" target="_blank">chandlerc@gmail.com</a>></span>:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="m_2315590147657139125m_-4391909161219543446gmail- gmail_msg"><div dir="ltr" class="gmail_msg">On Fri, Dec 30, 2016 at 2:08 AM Piotr Padlewski 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:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg">Thanks for very accurate responses.<div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg">- I totally agree what Dave and Chandler said about explicit and implicit operations, this is what I meant my first email.</div><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> I believe there are places like </div><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> v.emplace_back(A, B);</div><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> istead of</div><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> v.push_back(make_pair(A, B));b</div><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> That can make code simpler.</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">Do you have examples? The only ones i can come up with are the ones where the push_back variant literally can't compile because the type isn't movable.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Perhaps it would be useful to break down categories of can happen here...</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Case 1: there is one object already -- this is a *conversion* of a type.</div><div class="gmail_msg">- If the author of the conversion made it *implicit*, then 'v.push_back(x)' just works.</div><div class="gmail_msg">- If the author of the conversion made it *explicit* I would like to see the name of the type explicitly: 'v.push_back(T(x))'.</div><div class="gmail_msg"><br class="gmail_msg"></div></div></div></blockquote><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"></div><div class="gmail_msg">Case 2a: There is a collection of objects that are being composed into an aggregate. We don't have any interesting logic in the constructor, it takes an initializer list.</div><div class="gmail_msg">- This should work with 'v.push_back({a, b, c})'</div><div class="gmail_msg">- If it doesn't today, we can fix the type's constructors so that it does.</div><div class="gmail_msg">- Using 'emplace_back' doesn't help much -- you still need {}s to form the std::initializer_list in many cases. Pair and tuple are somewhat unusual in not requiring them.</div><div class="gmail_msg"><br class="gmail_msg"></div></div></div></blockquote></span><div class="gmail_msg"> This sounds extremely reasonable. </div><span class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"></div><div class="gmail_msg">Case 2b: A specific constructor needs to be called with an argument list. These arguments are not merely being aggregated but are inputs to a constructor that contains logic.</div><div class="gmail_msg">- This is analogous to a case called out w.r.t. '{...}' syntax in the coding standards[1]</div><div class="gmail_msg">- Similar to that rule, I would like to see a *call to the constructor* rather than hiding it behind 'emplace_back' as this is a function with interesting logic.</div><div class="gmail_msg">- That means i would write T(a, b, c) anyways, and 'v.push_back(T(a, b, c))' works.</div><div class="gmail_msg"><br class="gmail_msg"></div></div></div></blockquote></span><div class="gmail_msg">Calling emplace_back with 0 or multiple arguments is a clear way of saying "this constructor takes multiple arguments".</div><div class="gmail_msg">We can do it with initializer list with easy way like:</div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">v.emplace_back() == v.push_back({})</font></div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">v.emplace_back(a, b ,c) == v.push_back({a, b, c})</font></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I personally never liked the initializer syntax because of tricky casees like:</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">vector<string> v{{"abc", "def"}};</font><br class="gmail_msg"></div><div class="gmail_msg">Which is equivalent of </div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">vector<string> v = {std::string("abc", "def")};</font><br class="gmail_msg"></div><div class="gmail_msg">That will call std::string ctor with 2 iterators likely crashing, and putting same string might gives us empty string.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">In this case programmer probably meant</div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">std::vector<std:string> v({"abc", "def"});</font><br class="gmail_msg"></div><div class="gmail_msg">or</div><div class="gmail_msg"><font face="monospace, monospace" class="gmail_msg">std::vector<std::string> v = {"abc", "def"};</font><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">But this case is not possible to mess up with push_back (in the case of vector<vector<string>> or something). At least I hope it is not.</div><div class="gmail_msg">So avoiding braces is my personal preference. It is fine for me if we would choose to prefer '<font face="monospace, monospace" class="gmail_msg">v.push_back({a, b, c})</font>' instead of '<font face="monospace, monospace" class="gmail_msg">v.emplace_back(a, b, c)</font>', ofc as long as most of the community would prefer first form to the second :)</div><span class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"></div><div class="gmail_msg">[1]: <a href="http://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor" class="gmail_msg" target="_blank">http://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor</a></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Case 3: Passing objects of type 'T' through 'push_back' fails to compile because they cannot be copied or moved.</div><div class="gmail_msg">- You *must* use 'emplace_back' here. No argument (obviously).</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">My experience with LLVM code and other codebases is that case 3 should be extremely rare. The intersection of "types that cannot be moved or copied" and "types that you put into containers" is typically small.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Anyways, I don't disagree with this point with a tiny fix:</div><span class="m_2315590147657139125m_-4391909161219543446gmail- gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"><div class="m_2315590147657139125m_-4391909161219543446gmail-m_-7306051220478311565gmail_msg gmail_msg"> I think in cases like this we can leave it for judgement of contributor.</div></div></blockquote></span><div class="gmail_msg">*or reviewer*. ;]</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I continue to think exceptions can be made in rare cases when folks have good reasons. But I expect this to be quite rare. =]</div></div></div>
</blockquote></span></div><span class="m_2315590147657139125HOEnZb gmail_msg"><font color="#888888" class="gmail_msg"><br class="gmail_msg"></font></span></div><span class="m_2315590147657139125HOEnZb gmail_msg"><font color="#888888" class="gmail_msg"><div class="gmail_extra gmail_msg">Piotr</div></font></span></div>
</blockquote></div><br class="gmail_msg"></div>
_______________________________________________<br class="gmail_msg">
cfe-dev mailing list<br class="gmail_msg">
<a href="mailto:cfe-dev@lists.llvm.org" class="gmail_msg" target="_blank">cfe-dev@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br class="gmail_msg">
</blockquote></div></div>