lebedev.ri added inline comments.
================ Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:42-45 + if (Decl->getKind() != Decl::Kind::Namespace) + return false; + + auto const *NS = reinterpret_cast<NamespaceDecl const *>(Decl); ---------------- Use proper casting, `isa<>()`, `dyn_cast<>`, so on. ================ Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:79 + const MatchFinder::MatchResult &Result) { + NamespaceDecl const &ND = *Result.Nodes.getNodeAs<NamespaceDecl>("namespace"); + SourceManager const &Sources = *Result.SourceManager; ---------------- Here and everywhere - the consts are on the wrong side. ================ Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:102-108 + if (childrenCount(ND.decls()) == 0) { + SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace); + + diag(Namespaces.front().Begin, + "Nested namespaces are empty and can be removed", + DiagnosticIDs::Warning) + << FixItHint::CreateRemoval(Removal); ---------------- 1. This is unrelated to the main check, so it should be in a separate check. 2. This is really fragile i think. What about preprocessor-driven emptiness? ================ Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29-32 + struct NamespaceContext { + std::string Name; + SourceLocation Begin, NameBegin, RBrace; + }; ---------------- This looks like a pessimization. I think you should just store `NamespaceDecl*`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52136 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits