<div dir="ltr">Do the grep results seem to be the intended ones? (or are some of the comments unrelated to end-of-namespace comments that might then skew the data incorrectly?)<br><br>If so, then I think updating the coding guidelines to match predominant existing practice seems suitable. (maybe double check if there's been any history of that rule in the coding guidelines - maybe it used to be the "namespace" version and then changed, so perhaps we have a lot of outdated comments using a prior style? This is certainly true of naming, but I guess it's probably not true for this instance but could be worth checking)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 6, 2021 at 12:04 PM Carlos Galvez via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>I was recently working on a patch and was asked during review to replace existing:</div><div>"// end namespace clang"   Style A<br></div><div>with :<br></div><div>"// namespace clang"          Style B<br></div><div><br></div><div>After that, I got interested to understand what the preferred style is, and found in the <a href="https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions" target="_blank">Coding Guidelines</a> that the style is actually Style A.<br></div><div><br></div><div>On the other hand, clang-format will automatically enforce Style B on new code, via the FixNamespaceComments option, which is set to "true" for the LLVM style. clang-format will keep the Style A if it already exists, however. Most people using clang-format (outside LLVM) will probably be more familiar with Style B.</div><div><br></div><div>Additionally, I have seen the following usage numbers in the repo:</div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">$ git grep '//</span><em style="margin-top:0px;color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"> end' | wc -l<br style="margin-top:0px">6724<br>$ git grep '//</em><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"> namespace' | wc -l</span><br style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">14348</span><br></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"><br></span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">So Style B seems to be more adopted. Therefore I wanted to ask - should we update the Coding Guidelines to reflect this, and avoid these kinds of style discussions in code reviews? If so, what style should be preferred? I have a <a href="https://reviews.llvm.org/D115115" target="_blank">patch</a> for review and there seems to be a preference for keeping both styles. Regardless of the choice, I don't think this should lead to an urgent style change of the whole codebase.</span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"><br></span></div><div><font color="#000000" face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol, Lato, Helvetica Neue, Helvetica, Arial, sans-serif">Looking forward to your feedback!</font></div><div><font color="#000000" face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol, Lato, Helvetica Neue, Helvetica, Arial, sans-serif"><br></font></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">Best regards,</span></div><div><span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">Carlos</span></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>