r240270 - Fixed/added namespace ending comments using clang-tidy. NFC

[Alexander Kornienko]

On Mon, Jun 22, 2015 at 6:15 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Jun 22, 2015 at 2:47 AM, Alexander Kornienko <alexfh at 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 <http://llvm.org/docs/CodingStandards.html#namespace-indentation>,
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

This patch corresponds to 4b.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150623/c3d19d7d/attachment.html>