diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp --- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp +++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp @@ -8,8 +8,10 @@ #include "AnalysisInternal.h" #include "clang-include-cleaner/Types.h" +#include "clang/AST/ASTFwd.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" @@ -72,17 +74,25 @@ // Picks the most specific specialization for a // (Deduced)TemplateSpecializationType, while prioritizing using-decls. NamedDecl *getMostRelevantTemplatePattern(const T *TST) { - // This is the underlying decl used by TemplateSpecializationType, can be - // null when type is dependent. - auto *RD = TST->getAsTagDecl(); - auto *ND = resolveTemplateName(TST->getTemplateName()); // In case of exported template names always prefer the using-decl. This // implies we'll point at the using-decl even when there's an explicit // specializaiton using the exported name, but that's rare. + auto *ND = resolveTemplateName(TST->getTemplateName()); if (llvm::isa_and_present(ND)) return ND; - // Fallback to primary template for dependent instantiations. - return RD ? RD : ND; + // This is the underlying decl used by TemplateSpecializationType, can be + // null when type is dependent if so fallback to primary template. + CXXRecordDecl *TD = TST->getAsCXXRecordDecl(); + if (!TD) + return ND; + // We ignore explicit instantiations. This might imply marking the wrong + // declaration as used in specific cases, but seems like the right trade-off + // in general (e.g. we don't want to include a custom library that has an + // explicit specialization of a common type). + if (auto *Pat = TD->getTemplateInstantiationPattern()) + return Pat; + // For explicit specializations, use the specialized decl directly. + return TD; } public: @@ -182,6 +192,23 @@ return true; } + // Report a reference from explicit specializations to the specialized + // template. Implicit ones are filtered out by RAV and explicit instantiations + // are already traversed through typelocs. + bool + VisitClassTemplateSpecializationDecl(ClassTemplateSpecializationDecl *CTSD) { + if (CTSD->isExplicitSpecialization()) + report(CTSD->getLocation(), + CTSD->getSpecializedTemplate()->getTemplatedDecl()); + return true; + } + bool VisitVarTemplateSpecializationDecl(VarTemplateSpecializationDecl *VTSD) { + if (VTSD->isExplicitSpecialization()) + report(VTSD->getLocation(), + VTSD->getSpecializedTemplate()->getTemplatedDecl()); + return true; + } + // TypeLoc visitors. bool VisitUsingTypeLoc(UsingTypeLoc TL) { report(TL.getNameLoc(), TL.getFoundDecl()); diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -16,11 +16,11 @@ #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Testing/TestAST.h" #include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ScopedPrinter.h" @@ -28,6 +28,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include #include #include #include @@ -192,8 +193,7 @@ Pair(Code.point("1"), UnorderedElementsAre(HdrFile)), Pair(Code.point("2"), UnorderedElementsAre(HdrFile)), Pair(Code.point("3"), UnorderedElementsAre(MainFile)), - Pair(Code.point("4"), UnorderedElementsAre(MainFile)) - )); + Pair(Code.point("4"), UnorderedElementsAre(MainFile)))); } class AnalyzeTest : public testing::Test { @@ -433,5 +433,42 @@ Hinted(Hints::PreferredHeader)); } +// Test ast traversal & redecl selection end-to-end for templates, as explicit +// instantiations/specializations are not redecls of the primary template. We +// need to make sure we're selecting the right ones. +TEST_F(WalkUsedTest, TemplateDecls) { + llvm::Annotations Code(R"cpp( + #include "fwd.h" + #include "def.h" + #include "partial.h" + template <> struct $exp_spec^Foo {}; + template struct $exp^Foo; + $full^Foo x; + $implicit^Foo y; + $partial^Foo z; + )cpp"); + Inputs.Code = Code.code(); + Inputs.ExtraFiles["fwd.h"] = guard("template struct Foo;"); + Inputs.ExtraFiles["def.h"] = guard("template struct Foo {};"); + Inputs.ExtraFiles["partial.h"] = + guard("template struct Foo {};"); + TestAST AST(Inputs); + auto &SM = AST.sourceManager(); + const auto *Fwd = SM.getFileManager().getFile("fwd.h").get(); + const auto *Def = SM.getFileManager().getFile("def.h").get(); + const auto *Partial = SM.getFileManager().getFile("partial.h").get(); + + EXPECT_THAT( + offsetToProviders(AST, SM), + AllOf(Contains( + Pair(Code.point("exp_spec"), UnorderedElementsAre(Fwd, Def))), + Contains(Pair(Code.point("exp"), UnorderedElementsAre(Fwd, Def))), + Contains(Pair(Code.point("full"), UnorderedElementsAre(Fwd, Def))), + Contains( + Pair(Code.point("implicit"), UnorderedElementsAre(Fwd, Def))), + Contains( + Pair(Code.point("partial"), UnorderedElementsAre(Partial))))); +} + } // namespace } // namespace clang::include_cleaner diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp @@ -6,24 +6,34 @@ // //===----------------------------------------------------------------------===// #include "AnalysisInternal.h" +#include "clang-include-cleaner/Types.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceLocation.h" #include "clang/Frontend/TextDiagnostic.h" #include "clang/Testing/TestAST.h" -#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/GenericUniformityImpl.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/raw_ostream.h" #include "llvm/Testing/Annotations/Annotations.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include #include #include #include namespace clang::include_cleaner { namespace { +using testing::ElementsAre; // Specifies a test of which symbols are referenced by a piece of code. // Target should contain points annotated with the reference kind. @@ -31,7 +41,9 @@ // Target: int $explicit^foo(); // Referencing: int x = ^foo(); // There must be exactly one referencing location marked. -void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) { +// Returns target decls. +std::vector testWalk(llvm::StringRef TargetCode, + llvm::StringRef ReferencingCode) { llvm::Annotations Target(TargetCode); llvm::Annotations Referencing(ReferencingCode); @@ -51,6 +63,7 @@ FileID TargetFile = SM.translateFile( llvm::cantFail(AST.fileManager().getFileRef("target.h"))); + std::vector TargetDecls; // Perform the walk, and capture the offsets of the referenced targets. std::unordered_map> ReferencedOffsets; for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) { @@ -63,6 +76,7 @@ if (NDLoc.first != TargetFile) return; ReferencedOffsets[RT].push_back(NDLoc.second); + TargetDecls.push_back(&ND); }); } for (auto &Entry : ReferencedOffsets) @@ -94,6 +108,7 @@ // If there were any differences, we print the entire referencing code once. if (!DiagBuf.empty()) ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode; + return TargetDecls; } TEST(WalkAST, DeclRef) { @@ -114,25 +129,127 @@ // One explicit call from the TypeLoc in constructor spelling, another // implicit reference through the constructor call. testWalk("struct $explicit^$implicit^S { static int x; };", "auto y = ^S();"); - testWalk("template struct $explicit^Foo {};", "^Foo x;"); - testWalk(R"cpp( +} + +MATCHER_P(HasKind, Kind, "") { + if (arg->getKind() == Kind) + return true; + *result_listener << "Got kind: " << arg->getDeclKindName(); + return false; +} + +TEST(WalkAST, ClassTemplates) { + // Explicit instantiation and (partial) specialization references primary + // template. + EXPECT_THAT(testWalk("template struct $explicit^Foo{};", + "template struct ^Foo;"), + ElementsAre(HasKind(Decl::CXXRecord))); + EXPECT_THAT(testWalk("template struct $explicit^Foo{};", + "template<> struct ^Foo {};"), + ElementsAre(HasKind(Decl::CXXRecord))); + EXPECT_THAT(testWalk("template struct $explicit^Foo{};", + "template struct ^Foo {};"), + ElementsAre(HasKind(Decl::CXXRecord))); + + // Implicit instantiations references most relevant template. + EXPECT_THAT( + testWalk("template struct $explicit^Foo {};", "^Foo x;"), + ElementsAre(HasKind(Decl::CXXRecord))); + EXPECT_THAT(testWalk(R"cpp( template struct Foo {}; template<> struct $explicit^Foo {};)cpp", - "^Foo x;"); - testWalk(R"cpp( + "^Foo x;"), + ElementsAre(HasKind(Decl::ClassTemplateSpecialization))); + EXPECT_THAT(testWalk(R"cpp( template struct Foo {}; template struct $explicit^Foo { void x(); };)cpp", - "^Foo x;"); - testWalk(R"cpp( - template struct Foo {}; - template struct $explicit^Foo;)cpp", - "^Foo x;"); + "^Foo x;"), + ElementsAre(HasKind(Decl::ClassTemplatePartialSpecialization))); + EXPECT_THAT(testWalk(R"cpp( + template struct $explicit^Foo {}; + template struct Foo;)cpp", + "^Foo x;"), + ElementsAre(HasKind(Decl::CXXRecord))); // FIXME: This is broken due to // https://github.com/llvm/llvm-project/issues/42259. - testWalk(R"cpp( + EXPECT_THAT(testWalk(R"cpp( template struct $explicit^Foo { Foo(T); }; - template<> struct Foo { void get(); Foo(int); };)cpp", - "^Foo x(3);"); + template<> struct Foo { Foo(int); };)cpp", + "^Foo x(3);"), + ElementsAre(HasKind(Decl::ClassTemplate))); +} +TEST(WalkAST, VarTemplates) { + // Explicit instantiation and (partial) specialization references primary + // template. + // FIXME: Explicit instantiations has wrong source location, they point at the + // primary template location (hence we drop the reference). + EXPECT_THAT( + testWalk("template T Foo = 0;", "template int ^Foo;"), + ElementsAre()); + EXPECT_THAT(testWalk("template T $explicit^Foo = 0;", + "template<> int ^Foo = 2;"), + ElementsAre(HasKind(Decl::Var))); + EXPECT_THAT(testWalk("template T $explicit^Foo = 0;", + "template T* ^Foo = 1;"), + ElementsAre(HasKind(Decl::Var))); + + // Implicit instantiations references most relevant template. + // FIXME: This points at implicit specialization, instead we should point to + // pattern. + EXPECT_THAT(testWalk(R"cpp( + template T $explicit^Foo = 0;)cpp", + "int z = ^Foo;"), + ElementsAre(HasKind(Decl::VarTemplateSpecialization))); + EXPECT_THAT(testWalk(R"cpp( + template T Foo = 0; + template<> int $explicit^Foo = 1;)cpp", + "int x = ^Foo;"), + ElementsAre(HasKind(Decl::VarTemplateSpecialization))); + // FIXME: This points at implicit specialization, instead we should point to + // explicit partial specializaiton pattern. + EXPECT_THAT(testWalk(R"cpp( + template T Foo = 0; + template T* $explicit^Foo = nullptr;)cpp", + "int *x = ^Foo;"), + ElementsAre(HasKind(Decl::VarTemplateSpecialization))); + EXPECT_THAT(testWalk(R"cpp( + template T $explicit^Foo = 0; + template int Foo;)cpp", + "int x = ^Foo;"), + ElementsAre(HasKind(Decl::VarTemplateSpecialization))); +} +TEST(WalkAST, FunctionTemplates) { + // Explicit instantiation and (partial) specialization references primary + // template. + // FIXME: Explicit instantiations has wrong source location, they point at the + // primary template location (hence we drop the reference). + EXPECT_THAT(testWalk("template void foo(T) {}", + "template void ^foo(int);"), + ElementsAre()); + // FIXME: Report specialized template as used from explicit specializations. + EXPECT_THAT(testWalk("template void foo(T);", + "template<> void ^foo(int);"), + ElementsAre()); + EXPECT_THAT(testWalk("template void foo(T) {}", + "template void ^foo(T*) {}"), + ElementsAre()); + + // Implicit instantiations references most relevant template. + EXPECT_THAT(testWalk(R"cpp( + template void $explicit^foo() {})cpp", + "auto x = []{ ^foo(); };"), + ElementsAre(HasKind(Decl::FunctionTemplate))); + // FIXME: DeclRefExpr points at primary template, not the specialization. + EXPECT_THAT(testWalk(R"cpp( + template void $explicit^foo() {} + template<> void foo(){})cpp", + "auto x = []{ ^foo(); };"), + ElementsAre(HasKind(Decl::FunctionTemplate))); + EXPECT_THAT(testWalk(R"cpp( + template void $explicit^foo() {}; + template void foo();)cpp", + "auto x = [] { ^foo(); };"), + ElementsAre(HasKind(Decl::FunctionTemplate))); } TEST(WalkAST, Alias) { @@ -215,7 +332,7 @@ template S(T t) -> S; } using ns::$explicit^S;)cpp", - "^S x(123);"); + "^S x(123);"); testWalk("template struct $explicit^S {};", R"cpp( template