----- Original Message -----
> From: "Alexander Kornienko" <ale...@google.com>
> To: "David Blaikie" <dblai...@gmail.com>
> Cc: "llvm cfe" <cfe-commits@cs.uiuc.edu>
> Sent: Monday, June 22, 2015 5:38:59 PM
> Subject: Re: r240270 - Fixed/added namespace ending comments using
> clang-tidy. NFC
>
>
> On Mon, Jun 22, 2015 at 6:15 PM, David Blaikie < dblai...@gmail.com >
> wrote:
>
>
> On Mon, Jun 22, 2015 at 2:47 AM, Alexander Kornienko <
> ale...@google.com > wrote:
>
>
> Author: alexfh
> Date: Mon Jun 22 04:47:44 2015
> New Revision: 240270
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240270&view=rev
> Log:
> Fixed/added namespace ending comments using clang-tidy. NFC
>
>
> Is this actually an LLVM or Clang coding convention? It doesn't seem
> to be & I'm not sure it's one worth adopting... might be worth some
> discussion before we go in this direction? (perhaps there has been
> discussion in a review thread, etc & I've missed it - helpful to
> call that out in the commit message (& probably put this in the LLVM
> coding conventions if we are going in that direction))
>
>
>
> I was having an impression that there's a convention to use namespace
> ending comments of this form, that's why I considered a purely
> mechanical change that doesn't need a discussion. However, now
> reading the coding standards , I see that the relevant rule is much
> weaker, it just allows adding comments, not even recommends them:
>
>
>
>
>
>
> If it helps readability, feel free to add a comment indicating what
> namespace is being closed by a } .
>
> and also the style of the comment used in the example is slightly
> different:
>
>
>
>
> } // end namespace llvm
> instead of
>
>
>
> } // namespace llvm used in this patch.
>
>
> So an agreement on the style (and necessity) of the namespace ending
> comments prior to committing the patch wouldn't hurt. Would you like
> me to revert the patches before we figure out what the desired state
> is?
>
>
> Also, what the preferred solution here would be?
>
>
> 1. leave the comments (or the lack thereof) alone, don't even fix
> incorrect or inconsistent ones
> 2. only fix incorrect comments
> 3. fix incorrect comments and make all existing namespace ending
> comments consistent
> 4. 3. + add namespace ending comments to all namespaces spanning more
> than X lines
> 5. something else?
>
>
> In cases 2-4 we need to decide which format of namespace ending
> comments to use:
> a. // llvm
> b. // namespace llvm
> c. // end namespace llvm
> d. // end of namespace llvm
> e. // end llvm namespace
> f. something else
>
Also, I prefer that anonymous namespaces are explicitly commented as such if
they're going to be commented, and we also don't generally add indentation for
anonymous namespaces. This change added comments like:
} // namespace
and I'd rather it said:
// anonymous namespace
Regarding the actual format, putting 'end %s namespace' seems to be preferred:
$ grep -r 'anonymous namespace' lib | sed 's/.*://' | sed 's/[ ]\+/ /g' | sort
| uniq -c
16 } // anonymous namespace
2 } // anonymous namespace.
134 } // end anonymous namespace
15 } // end anonymous namespace.
2 } //end anonymous namespace
18 } // End anonymous namespace
11 } // End anonymous namespace.
1 } //End anonymous namespace
13 } // end of anonymous namespace
8 } // End of anonymous namespace
1 } // End of anonymous namespace.
-Hal
>
> This patch corresponds to 4b.
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
