[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

Wojtek GumuĊ‚a via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 25 10:43:24 PDT 2018


wgml added a comment.

Thanks you all for your participation in the review process. I guess it is done now.
I don't have write access to the repositories, @aaron.ballman, could you apply the patch on my behalf?



================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:31
+static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
+  const NamespaceDecl::decl_range Decls = ND.decls();
+  if (std::distance(Decls.begin(), Decls.end()) != 1)
----------------
aaron.ballman wrote:
> wgml wrote:
> > aaron.ballman wrote:
> > > We usually only const-qualify local declarations when they're pointers/references, so you can drop the `const` here (and in several other places). It's not a hard and fast rule, but the clutter is not useful in such small functions.
> > From my perspective, `const` is a easy way of declaring that my intention is only to name given declaration only for reading and to improve code readability. 
> I wasn't making a value judgement about the coding style so much as pointing out that this is novel to the code base and we tend to avoid novel constructs unless there's a good reason to deviate. I don't see a strong justification here, so I'd prefer them to be removed for consistency.
Okay then, adjusted.


https://reviews.llvm.org/D52136





More information about the cfe-commits mailing list