<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 9, 2014 at 2:58 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><div>On Oct 9, 2014, at 10:47 AM, Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:</div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 8, 2014 at 7:20 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br></div><div class="gmail_quote"><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="word-wrap:break-word"><span><blockquote type="cite"><div dir="ltr"><div class="gmail_extra">Sure, I actually have no problem with this.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I'm going to point out that one of the naming conventions used by LLD has serious problems: naming variables with a leading underscore puts them *way* too close to the reserved identifier space. Folks have accidentally ended up with reserved names quite a few times already.</div></div></blockquote></span><div>The lld conventions for ivars is a leading underscore followed by a lowercase letter. The reserved identifiers are a leading underscore followed by an uppercase letter. There is no conflict. </div><div><br></div><div>On the other hand the LLVM conventions prevents the use of the -Wshadow which catches real bugs.</div></div></blockquote><div><br></div><div>If people think this is a valuable warning, we should just fix it so that it doesn't fire on this common pattern:</div><div>struct A {</div><div> A(int x) : x(x) {}</div><div> int x;<br>};</div><div><br></div><div>It should still fire on this pattern though:</div><div><div>struct A {</div><div> A(int x) : x(x) { x++; } // I modified the parameter 'x', not the member 'x'.</div><div> int x;<br>};</div></div><div><br></div><div>We shouldn't create conventions just to work around bugs in a toolchain that we control.</div></div></div></div></blockquote></span><div>Or if your convention does not work with existing tools (clang, gcc, VisualStudio, etc), maybe that should be a hint that the convention is buggy.</div></div></div></blockquote><div><br>I don't really agree with this. The idiom of naming ctor parameters the same as members isn't uncommon - one reason -Wshadow isn't on by default, because large codebases don't conform to it so the signal/noise ratio is low.<br><br>There are quite a few cases where we improve Clang based on usage in LLVM, or disable VS warnings rather than working around them in LLVM code.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><div><br></div><div><br></div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>My two cents: LLVM's conventions around method names and local variable names are widely ignored. For example, the IRBuilder uses leading capitals for methods. Clang CodeGen does as well. Clang CodeGen also uses lots of local variables with leading lower-case letters, which is against the variable naming guidelines.</div><div><br></div><div>I think the takeaway from that is that lots of people don't actually understand or agree with the LLVM naming standards in this area. I don't think we should do a mass rename in LLD if we aren't willing to do a mass rename across Clang internals, and so far no one has indicated a desire to do that.</div></div></div></div></blockquote></span>Yes. The top of the LLVM coding conventions, the “golden rule” is don’t do mass renames. So, I’m surprised at the desire for mass renamings.</div><div><br></div><div>But, as you said, there is lots of straying from the naming conventions. </div></div></blockquote><div><br></div><div>So far as I know it's legacy (we didn't do a mass rename, so there's lots of old code still using prior conventions), consistency (use the local convention or at least change a whole file/class together - sometimes hard to get the momentum for that), or just simple mistakes (due to the inconsistency it's sometimes hard to remember the conventions), not deliberately going against the style conventions.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>So, as Chris suggested, I’ll start a thread to discuss naming conventions.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-Nick</div><div><br></div><div><br></div></font></span></div><br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div><br></div></div>