<div dir="ltr">I'm referring specifically to this part<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><br></div>But I'm not sure the key question here is what James said :-P I agree that reviewers should not pay any attention to this, but it would be nice if the coding guidelines conformed to whatever clang-format does (which doesn't even need to be explicit for this rule. It can just be a catch-all "do what clang-format says"). I think the coding examples are non-normative, so *shouldn't* be used as evidence of unrelated rules, but obviously people do actually read them that way, so formatting them with clang-format seems like a good idea.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 6, 2021 at 2:59 PM Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.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>
<p>Er, you and I are reading James' response very differently. <br>
</p>
<p>To me, this topic does not matter. We do not need to enforce a
rule on this. Reviewers should not be wasting time on this. <br>
</p>
<p>Philip<br>
</p>
<div>On 12/6/21 2:04 PM, Geoffrey
Martin-Noble wrote:<br>
</div>
<blockquote type="cite">
<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>
</blockquote></div>