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

Tue Dec 7 02:06:12 PST 2021

In case it helps, this graph shows the actual distribution of both styles
(in %), using the regex suggested by Geoffrey above. I personally thing
this is just something clang-format should update automatically as files
get modified - over time the style automatically becomes consistent. But
that's another story, I'm mostly interested in how to improve the Coding
Guidelines at this point.
[image: llvm_style.png]
Best regards,
Carlos

On Tue, Dec 7, 2021 at 12:05 AM Geoffrey Martin-Noble <gcmn at google.com>
wrote:

> I'm referring specifically to this part
>
> > That said, we should update the coding guidelines to recommend the
> format which clang-tidy emits -- just to make everyone's lives easier.
>
> 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.
>
> On Mon, Dec 6, 2021 at 2:59 PM Philip Reames <listmail at philipreames.com>
> wrote:
>
>> 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> 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> 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
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> 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/20211207/2c01fc70/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm_style.png
Type: image/png
Size: 29802 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211207/2c01fc70/attachment-0001.png>