aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:26 void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + using readability::NamespaceCommentCheck; + ---------------- I would rather use the fully-qualified names below -- the namespaces are actually of interest when needing to see what checks rely on what other modules quickly. ================ Comment at: clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:45 + if (const auto *Namespace = dyn_cast<NamespaceDecl>(Context)) + if (Namespace->getQualifiedNameAsString() == "llvm") + Replacement = "Register"; ---------------- dsanders wrote: > aaron.ballman wrote: > > This is a fairly expensive operation compared to calling `getName()`; are > > there situations where you think you need the extra work to be done? (Same > > comment below.) > None that are likely to occur. `getName()` with some way to check that > there's no enclosing namespace would be equivalent. Is there a cheap way to > tell if a namespace is at the top level? You could check if its `DeclContext` is the translation unit, I believe. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65919/new/ https://reviews.llvm.org/D65919 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits