[include-cleaner] Improve handling for templates

Principal here is:
- Making sure each template instantiation implies use of the most specialized
  template. As explicit instantiations/specializations are not redeclarations of
  the primary template.
- Introducing a use from explicit instantions/specializaitons to the primary
  template, as they're required but not traversed as part of the RAV.

Differential Revision: https://reviews.llvm.org/D148112

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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
   // 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<UsingShadowDecl, TypeAliasTemplateDecl>(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 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
     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());

clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

@@ -16,18 +16,19 @@
 #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"
 #include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
+#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -192,8 +193,7 @@ TEST_F(WalkUsedTest, MacroRefs) {
           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 @@ TEST(Hints, Ordering) {
             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<char> {};
+    template struct $exp^Foo<int>;
+    $full^Foo<int> x;
+    $implicit^Foo<bool> y;
+    $partial^Foo<int*> z;
+  )cpp");
+  Inputs.Code = Code.code();
+  Inputs.ExtraFiles["fwd.h"] = guard("template<typename> struct Foo;");
+  Inputs.ExtraFiles["def.h"] = guard("template<typename> struct Foo {};");
+  Inputs.ExtraFiles["partial.h"] =
+      guard("template<typename T> struct Foo<T*> {};");
+  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

clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

@@ -6,32 +6,44 @@
 //
 //===----------------------------------------------------------------------===//
 #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 <cstddef>
+#include <string>
 #include <unordered_map>
 #include <utility>
 #include <vector>
 
 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.
 // Example:
 //   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<NamedDecl *> testWalk(llvm::StringRef TargetCode,
+                                  llvm::StringRef ReferencingCode) {
   llvm::Annotations Target(TargetCode);
   llvm::Annotations Referencing(ReferencingCode);
 
@@ -51,6 +63,7 @@ void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
   FileID TargetFile = SM.translateFile(
       llvm::cantFail(AST.fileManager().getFileRef("target.h")));
 
+  std::vector<NamedDecl *> TargetDecls;
   // Perform the walk, and capture the offsets of the referenced targets.
   std::unordered_map<RefType, std::vector<size_t>> ReferencedOffsets;
   for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
@@ -63,6 +76,7 @@ void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
       if (NDLoc.first != TargetFile)
         return;
       ReferencedOffsets[RT].push_back(NDLoc.second);
+      TargetDecls.push_back(&ND);
     });
   }
   for (auto &Entry : ReferencedOffsets)
@@ -94,6 +108,7 @@ void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
   // 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 @@ TEST(WalkAST, TagType) {
   // 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<typename> struct $explicit^Foo {};", "^Foo<int> 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<typename> struct $explicit^Foo{};",
+                       "template struct ^Foo<int>;"),
+              ElementsAre(HasKind(Decl::CXXRecord)));
+  EXPECT_THAT(testWalk("template<typename> struct $explicit^Foo{};",
+                       "template<> struct ^Foo<int> {};"),
+              ElementsAre(HasKind(Decl::CXXRecord)));
+  EXPECT_THAT(testWalk("template<typename> struct $explicit^Foo{};",
+                       "template<typename T> struct ^Foo<T*> {};"),
+              ElementsAre(HasKind(Decl::CXXRecord)));
+
+  // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+      testWalk("template<typename> struct $explicit^Foo {};", "^Foo<int> x;"),
+      ElementsAre(HasKind(Decl::CXXRecord)));
+  EXPECT_THAT(testWalk(R"cpp(
     template<typename> struct Foo {};
     template<> struct $explicit^Foo<int> {};)cpp",
-           "^Foo<int> x;");
-  testWalk(R"cpp(
+                       "^Foo<int> x;"),
+              ElementsAre(HasKind(Decl::ClassTemplateSpecialization)));
+  EXPECT_THAT(testWalk(R"cpp(
     template<typename> struct Foo {};
     template<typename T> struct $explicit^Foo<T*> { void x(); };)cpp",
-           "^Foo<int *> x;");
-  testWalk(R"cpp(
-    template<typename> struct Foo {};
-    template struct $explicit^Foo<int>;)cpp",
-           "^Foo<int> x;");
+                       "^Foo<int *> x;"),
+              ElementsAre(HasKind(Decl::ClassTemplatePartialSpecialization)));
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename> struct $explicit^Foo {};
+    template struct Foo<int>;)cpp",
+                       "^Foo<int> 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<typename T> struct $explicit^Foo { Foo(T); };
-    template<> struct Foo<int> { void get(); Foo(int); };)cpp",
-           "^Foo x(3);");
+    template<> struct Foo<int> { 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<typename T> T Foo = 0;", "template int ^Foo<int>;"),
+      ElementsAre());
+  EXPECT_THAT(testWalk("template<typename T> T $explicit^Foo = 0;",
+                       "template<> int ^Foo<int> = 2;"),
+              ElementsAre(HasKind(Decl::Var)));
+  EXPECT_THAT(testWalk("template<typename T> T $explicit^Foo = 0;",
+                       "template<typename T> T* ^Foo<T*> = 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 <typename T> T $explicit^Foo = 0;)cpp",
+                       "int z = ^Foo<int>;"),
+              ElementsAre(HasKind(Decl::VarTemplateSpecialization)));
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> T Foo = 0;
+    template<> int $explicit^Foo<int> = 1;)cpp",
+                       "int x = ^Foo<int>;"),
+              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<typename T> T Foo = 0;
+    template<typename T> T* $explicit^Foo<T*> = nullptr;)cpp",
+                       "int *x = ^Foo<int *>;"),
+              ElementsAre(HasKind(Decl::VarTemplateSpecialization)));
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> T $explicit^Foo = 0;
+    template int Foo<int>;)cpp",
+                       "int x = ^Foo<int>;"),
+              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<typename T> void foo(T) {}",
+                       "template void ^foo<int>(int);"),
+              ElementsAre());
+  // FIXME: Report specialized template as used from explicit specializations.
+  EXPECT_THAT(testWalk("template<typename T> void foo(T);",
+                       "template<> void ^foo<int>(int);"),
+              ElementsAre());
+  EXPECT_THAT(testWalk("template<typename T> void foo(T) {}",
+                       "template<typename T> void ^foo(T*) {}"),
+              ElementsAre());
+
+  // Implicit instantiations references most relevant template.
+  EXPECT_THAT(testWalk(R"cpp(
+    template <typename T> void $explicit^foo() {})cpp",
+                       "auto x = []{ ^foo<int>(); };"),
+              ElementsAre(HasKind(Decl::FunctionTemplate)));
+  // FIXME: DeclRefExpr points at primary template, not the specialization.
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> void $explicit^foo() {}
+    template<> void foo<int>(){})cpp",
+                       "auto x = []{ ^foo<int>(); };"),
+              ElementsAre(HasKind(Decl::FunctionTemplate)));
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> void $explicit^foo() {};
+    template void foo<int>();)cpp",
+                       "auto x = [] { ^foo<int>(); };"),
+              ElementsAre(HasKind(Decl::FunctionTemplate)));
 }
 
 TEST(WalkAST, Alias) {
@@ -215,7 +332,7 @@ TEST(WalkAST, TemplateNames) {
         template <typename T> S(T t) -> S<T>;
       }
       using ns::$explicit^S;)cpp",
-      "^S x(123);");
+           "^S x(123);");
   testWalk("template<typename> struct $explicit^S {};",
            R"cpp(
       template <template <typename> typename> struct X {};