PiotrZSL marked 3 inline comments as done.
PiotrZSL added inline comments.
================
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),
----------------
njames93 wrote:
> 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?
Habit, hash_combine sounded better here, I can change this to +.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:201
- addUsage(Decl->getParent(), Decl->getNameInfo().getSourceRange(),
- Result.SourceManager);
+ bool shouldVisitTemplateInstantiations() const { return true; }
+
----------------
njames93 wrote:
> What is the reason for visiting template instantiations?
Tests were failing, some method overrides are known only when we visit template
instances. Without this code related to "Fix overridden methods" (line 281)
were not executed properly.
And as an result overridden virtual methods in template classes were not
"fixed". Previously it worked in same way.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:495
+ AggressiveDependentMemberLookup);
+ Visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(Decl));
}
----------------
njames93 wrote:
> Instead of this nasty const cast, you can just use
> `Visitor.TraverseAST(*Result.Context);`
Yes I know, but it looked strange when we register matcher for something, and
then in check we don't use that something.
I can change this.
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
