hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:81 + // specializaiton using the exported name, but that's rare. + if (llvm::isa<UsingShadowDecl, TypeAliasTemplateDecl>(ND) || !RD) + return ND; ---------------- I think we should be careful about the case where `ND == nullptr`, `isa` will trigger an assertion, use `isa_and_present`. ================ Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:82 + if (llvm::isa<UsingShadowDecl, TypeAliasTemplateDecl>(ND) || !RD) + return ND; + return RD; ---------------- We seem to miss a testcase for `TypeAliasTemplateDecl`, add one? ================ Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:119 + testWalk(R"cpp( + template<typename> struct Foo {}; + template<> struct $explicit^Foo<int> {};)cpp", ---------------- nit: The indentation looks weird, the `template...` should not align with the above `testWalk` text, would be better to add 4 space indentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146732/new/ https://reviews.llvm.org/D146732 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits