[PATCH] D149723: [clang-tidy] Optimize performance of RenamerClangTidyCheck

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

Mostly looks good, just a few small nits



================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:47
 
-    std::hash<NamingCheckId::second_type> SecondHash;
-    return DenseMapInfo<clang::SourceLocation>::getHashValue(Val.first) +
-           SecondHash(Val.second);
+    return hash_combine(
+        DenseMapInfo<clang::SourceLocation>::getHashValue(Val.first),
----------------
What's the purpose of using hash_combine instead of just adding the 2 hashes together, there is enough entropy in the hashes that a simple operation makes more sense?


================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:201
 
-    addUsage(Decl->getParent(), Decl->getNameInfo().getSourceRange(),
-             Result.SourceManager);
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
----------------
What is the reason for visiting template instantiations?


================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:495
+                                  AggressiveDependentMemberLookup);
+  Visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(Decl));
 }
----------------
Instead of this nasty const cast, you can just use `Visitor.TraverseAST(*Result.Context);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149723