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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
