[PATCH] D115115: [doc] Fix namespace comment style in Coding Guidelines

Carlos Galvez via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 6 00:11:58 PST 2021


carlosgalvezp added a comment.

In D115115#3172329 <https://reviews.llvm.org/D115115#3172329>, @salman-javed-nz wrote:

> The ~6700 instances of `// end namespace` - do you observe any trend, e.g. are they concentrated in one of the LLVM sub-projects? Is it similar to the situation we have with the `readability-identifier-naming` check, where a number of sub-projects have disabled it in their sub-directory's `.clang-tidy` file?

I see them pretty much in all sub-projects, mostly in clang and llvm due to them being the bigger sub-projects. But both styles are used on all sub-projects (the latter style used more frequently).

Unlike `.clang-tidy` files, I cannot see any `.clang-format` that sets `FixNamespaceComments = false`. The clang-format LLVM style hardcodes the value to true:

  clang/lib/Format/Format.cpp:  LLVMStyle.FixNamespaceComments = true;

This initiative comes from a previous review where I got feedback that the file I was updating (`GlobList.h` I think) had "incorrect" namespace comments, so I fixed it. Now I looked at the whole `clang-tidy` folder and noticed the problem exists in a few more files (not in the checkers, only in the "infrastructure" code). So this got me wondering and thought it would be good to read the Coding Guidelines, and sure enough the old style is there. Since it seems people consider that style "wrong" I have decided to put up this patch to update, so that we don't run into discussions like "I know this style is wrong, but the Coding Guidelines say otherwise".

Probably clang-format doesn't fix existing comments, that's why they remain. My guess as a newcomer is that first there was the old style, then clang-format came and set the new style, while not fixing the old style automatically.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115115/new/

https://reviews.llvm.org/D115115



More information about the llvm-commits mailing list