[llvm-dev] [docs][RFC] Style for "end namespace" comments

Philip Reames via llvm-dev llvm-dev at lists.llvm.org
Mon Dec 6 14:59:18 PST 2021


Er, you and I are reading James' response very differently.

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.

Philip

On 12/6/21 2:04 PM, Geoffrey Martin-Noble wrote:
> 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.
>
> On Mon, Dec 6, 2021 at 1:31 PM Philip Reames via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>
>     I agree with James.  Both are reasonable, this doesn't really
>     matter, we don't have to pick and enforce one.
>
>     Philip
>
>     On 12/6/21 12:47 PM, James Y Knight via llvm-dev wrote:
>>     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.
>>
>>     That said, we should update the coding guidelines to recommend
>>     the format which clang-tidy emits -- just to make everyone's
>>     lives easier.
>>
>>
>>
>>     On Mon, Dec 6, 2021 at 3:03 PM Carlos Galvez via llvm-dev
>>     <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>
>>         Hi,
>>
>>         I was recently working on a patch and was asked during review
>>         to replace existing:
>>         "// end namespace clang"   Style A
>>         with :
>>         "// namespace clang"          Style B
>>
>>         After that, I got interested to understand what the preferred
>>         style is, and found in the Coding Guidelines
>>         <https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions>
>>         that the style is actually Style A.
>>
>>         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.
>>
>>         Additionally, I have seen the following usage numbers in the
>>         repo:
>>
>>         $ git grep '/// end' | wc -l
>>         6724
>>         $ git grep '/// namespace' | wc -l
>>         14348
>>
>>         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 patch
>>         <https://reviews.llvm.org/D115115> 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.
>>
>>         Looking forward to your feedback!
>>
>>         Best regards,
>>         Carlos
>>         _______________________________________________
>>         LLVM Developers mailing list
>>         llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>>         https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>         <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>
>>
>>     _______________________________________________
>>     LLVM Developers mailing list
>>     llvm-dev at lists.llvm.org  <mailto:llvm-dev at lists.llvm.org>
>>     https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev  <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>     _______________________________________________
>     LLVM Developers mailing list
>     llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>     https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>     <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211206/914650cc/attachment.html>


More information about the llvm-dev mailing list