<div dir="ltr">And slightly different numbers (though I think it doesn't change any conclusions):<div><br></div><div><font face="monospace">$ git grep -E '}\s+//\s+namespace' | wc -l<br>14030<br><br>$ git grep -E '}\s+//\s+end\s+namespace' | wc -l<br>4185</font><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 6, 2021 at 2:04 PM Geoffrey Martin-Noble <<a href="mailto:gcmn@google.com">gcmn@google.com</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">I think James basically said the opposite: that we should use whatever clang-format does and be done with it. Things that don't really matter are one of the best cases for a style guide (accompanied by tooling to do it automatically). Otherwise you get reviewers commenting as they did on Carlos' patch. I would recommend that we encourage people to not comment on such things, but having it tool enforced and consistent seems good.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 6, 2021 at 1:31 PM Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">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>
<p>I agree with James. Both are reasonable, this doesn't really
matter, we don't have to pick and enforce one.</p>
<p>Philip<br>
</p>
<div>On 12/6/21 12:47 PM, James Y Knight via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Both styles accomplish the goal of annotating what
namespace is being closed -- and both are widely used within the
codebase. So I think there's not an intrinsic reason to prefer
one over the other. They're both fine.
<div><br>
</div>
<div>That said, we should update the coding guidelines to
recommend the format which clang-tidy emits -- just to make
everyone's lives easier.</div>
<div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Mon, Dec 6, 2021 at 3:03 PM
Carlos Galvez via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">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><br>
</span></div>
<div><span>$
git grep '//</span><em> end'
| wc -l<br style="margin-top:0px">
6724<br>
$ git grep '//</em><span> namespace'
| wc -l</span><br>
<span>14348</span><br>
</div>
<div><span><br>
</span></div>
<div><span>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><br>
</span></div>
<div><font face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol,
Lato, Helvetica Neue, Helvetica, Arial, sans-serif" color="#000000">Looking forward to your feedback!</font></div>
<div><font face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol,
Lato, Helvetica Neue, Helvetica, Arial, sans-serif" color="#000000"><br>
</font></div>
<div><span>Best
regards,</span></div>
<div><span>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>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
LLVM Developers mailing list
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
</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>
</blockquote></div>