<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<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 class="moz-cite-prefix">On 12/6/21 12:47 PM, James Y Knight via
llvm-dev wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAA2zVHrz9n-uSNkjsjFYk5ESZ2aarQsjha4gPTQVu7iD46x-VQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<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" moz-do-not-send="true">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" moz-do-not-send="true">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" moz-do-not-send="true">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 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 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"
moz-do-not-send="true">llvm-dev@lists.llvm.org</a><br>
<a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
</body>
</html>