[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 05:45:26 PDT 2019

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list