<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>