kadircet created this revision.
Herald added a subscriber: mgrang.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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 instantiions/specializaitons to the primary 
template, as they're required but not traversed as part of the RAV.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148112

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -6,24 +6,32 @@
 //
 //===----------------------------------------------------------------------===//
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.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.
@@ -31,7 +39,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 decl kinds.
+std::vector<std::string> testWalk(llvm::StringRef TargetCode,
+                                  llvm::StringRef ReferencingCode) {
   llvm::Annotations Target(TargetCode);
   llvm::Annotations Referencing(ReferencingCode);
 
@@ -51,6 +61,7 @@
   FileID TargetFile = SM.translateFile(
       llvm::cantFail(AST.fileManager().getFileRef("target.h")));
 
+  std::vector<std::string> TargetDeclKinds;
   // 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 +74,7 @@
       if (NDLoc.first != TargetFile)
         return;
       ReferencedOffsets[RT].push_back(NDLoc.second);
+      TargetDeclKinds.push_back(ND.getDeclKindName());
     });
   }
   for (auto &Entry : ReferencedOffsets)
@@ -94,6 +106,9 @@
   // If there were any differences, we print the entire referencing code once.
   if (!DiagBuf.empty())
     ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode;
+  llvm::sort(TargetDeclKinds);
+  TargetDeclKinds.erase(llvm::unique(TargetDeclKinds), TargetDeclKinds.end());
+  return TargetDeclKinds;
 }
 
 TEST(WalkAST, DeclRef) {
@@ -114,25 +129,123 @@
   // 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(
+}
+
+TEST(WalkAST, ClassTemplates) {
+  // Explicit instantiation and (partial) specialization references primary
+  // template.
+  EXPECT_THAT(testWalk("template<typename> struct $explicit^Foo{};",
+                       "template struct ^Foo<int>;"),
+              ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template<typename> struct $explicit^Foo{};",
+                       "template<> struct ^Foo<int> {};"),
+              ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template<typename> struct $explicit^Foo{};",
+                       "template<typename T> struct ^Foo<T*> {};"),
+              ElementsAre("ClassTemplate"));
+
+  // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+      testWalk("template<typename> struct $explicit^Foo {};", "^Foo<int> x;"),
+      ElementsAre("ClassTemplate"));
+  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("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(
+                       "^Foo<int *> x;"),
+              ElementsAre("ClassTemplatePartialSpecialization"));
+  EXPECT_THAT(testWalk(R"cpp(
     template<typename> struct Foo {};
     template struct $explicit^Foo<int>;)cpp",
-           "^Foo<int> x;");
+                       "^Foo<int> x;"),
+              ElementsAre("ClassTemplateSpecialization"));
   // 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("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("VarTemplate"));
+  EXPECT_THAT(testWalk("template<typename T> T $explicit^Foo = 0;",
+                       "template<typename T> T* ^Foo<T*> = 1;"),
+              ElementsAre("VarTemplate"));
+
+  // 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("VarTemplateSpecialization"));
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> T Foo = 0;
+    template<> int $explicit^Foo<int> = 1;)cpp",
+                       "int x = ^Foo<int>;"),
+              ElementsAre("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("VarTemplateSpecialization"));
+  // FIXME: Should pick the explicit instantiaion instead.
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> T $explicit^Foo = 0;
+    template int Foo<int>;)cpp",
+                       "int x = ^Foo<int>;"),
+              ElementsAre("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("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("FunctionTemplate"));
+  // FIXME: DeclRefExpr points at primary template, not the explicit
+  // instantiation.
+  EXPECT_THAT(testWalk(R"cpp(
+    template<typename T> void $explicit^foo() {};
+    template void foo<int>();)cpp",
+                       "auto x = [] { ^foo<int>(); };"),
+              ElementsAre("FunctionTemplate"));
 }
 
 TEST(WalkAST, Alias) {
@@ -215,7 +328,7 @@
         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 {};
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ 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 <cstddef>
+#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -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,43 @@
             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();
+  const auto *Main = SM.getFileEntryForID(SM.getMainFileID());
+
+  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(Main))),
+            Contains(
+                Pair(Code.point("implicit"), UnorderedElementsAre(Fwd, Def))),
+            Contains(
+                Pair(Code.point("partial"), UnorderedElementsAre(Partial)))));
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ 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<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.
+    TagDecl *TD = TST->getAsTagDecl();
+    if (!TD)
+      return ND;
+    auto *CTSD = llvm::cast<ClassTemplateSpecializationDecl>(TD);
+    if (CTSD->isExplicitInstantiationOrSpecialization())
+      return CTSD;
+    auto Specializes = CTSD->getSpecializedTemplateOrPartial();
+    if (auto *Partial =
+            Specializes.dyn_cast<ClassTemplatePartialSpecializationDecl *>())
+      return Partial;
+    return Specializes.get<ClassTemplateDecl *>();
   }
 
 public:
@@ -182,6 +192,18 @@
     return true;
   }
 
+  // Report a reference from explicit specializations/instantiations to the
+  // specialized template.
+  bool
+  VisitClassTemplateSpecializationDecl(ClassTemplateSpecializationDecl *CTSD) {
+    report(CTSD->getLocation(), CTSD->getSpecializedTemplate());
+    return true;
+  }
+  bool VisitVarTemplateSpecializationDecl(VarTemplateSpecializationDecl *VTSD) {
+    report(VTSD->getLocation(), VTSD->getSpecializedTemplate());
+    return true;
+  }
+
   // TypeLoc visitors.
   bool VisitUsingTypeLoc(UsingTypeLoc TL) {
     report(TL.getNameLoc(), TL.getFoundDecl());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to