[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet added a comment.

caea93cc4478cca28321cba4fa2871d5e6090299 
 should 
fix the issue


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

https://reviews.llvm.org/D148112

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Çetinkaya via cfe-commits 
figured out the issue, preparing a fix

On Wed, Apr 12, 2023 at 7:46 PM Douglas Yung via Phabricator <
revi...@reviews.llvm.org> wrote:

> dyung added a comment.
>
> @kadircet, your change is causing 3 test failures on Windows bots, can you
> take a look and revert if you need time to investigate?
>
> https://lab.llvm.org/buildbot/#/builders/216/builds/19740
> https://lab.llvm.org/buildbot/#/builders/123/builds/18332
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D148112/new/
>
> https://reviews.llvm.org/D148112
>
>

-- 
Want to support earthquake

victims in Turkey & Syria?
- https://google.benevity.org/campaigns/13999
- https://google.benevity.org/campaigns/14045
- https://google.benevity.org/campaigns/14018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Douglas Yung via Phabricator via cfe-commits 
dyung added a comment.

@kadircet, your change is causing 3 test failures on Windows bots, can you take 
a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/216/builds/19740
https://lab.llvm.org/buildbot/#/builders/123/builds/18332


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

https://reviews.llvm.org/D148112

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG34f5774920f5: [include-cleaner] Improve handling for 
templates (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

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,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();
 });
   }
   for (auto  : 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

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet marked 3 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:109
 ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode;
+  llvm::sort(TargetDeclKinds);
+  TargetDeclKinds.erase(llvm::unique(TargetDeclKinds), TargetDeclKinds.end());

sammccall wrote:
> this seems a little ad-hoc (is class name always enough info, why are we 
> sorting/uniquing here)
> 
> Consider returning vector and using a matcher to verify the 
> things you care about (`kind(Decl::ClassTemplate)` or so?)
sorry was planning to drop this actually, analysistest covers the "we're 
picking the right declaration at a certain location" logic. but since i forgot, 
i might as well change it to keep decls around :P


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

https://reviews.llvm.org/D148112

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet updated this revision to Diff 512851.
kadircet marked an inline comment as done.
kadircet added a comment.
Herald added a subscriber: ChuanqiXu.

- Ignore explicit instantiations
- Update tests to use decls


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

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,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();
 });
   }
   for (auto  : 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

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Sam McCall via Phabricator via cfe-commits 
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:91
+auto *CTSD = llvm::cast(TD);
+if (CTSD->isExplicitInstantiationOrSpecialization())
+  return CTSD;

We could consider the other order:

```
if (auto *Pattern = TD->getTemplateInstantiationPattern())
  return Pattern;
return TD;
```

To me this feels a bit more like there's an obvious principle and less like 
case bashing.

Concretely I think the difference is that we now report the pattern rather than 
the specialization for explicit template instantiations. As discussed offline, 
I'm not very familiar with the patterns where explicit instantiations are used. 
But it seems at least ambiguous whether a visible explicit instantiation is 
semantically significant, whereas it's clear that the pattern is important. So 
I think preferring the pattern here would be slightly better too, up to you.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:194
+  // Report a reference from explicit specializations/instantiations to the
+  // specialized template.
+  bool

maybe add to the comment that implicit ones are never visited?
(It should have been obvious to me, but it's easy to fall into the trap of 
"what are all the things a CTSD can be again?")



Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:109
 ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode;
+  llvm::sort(TargetDeclKinds);
+  TargetDeclKinds.erase(llvm::unique(TargetDeclKinds), TargetDeclKinds.end());

this seems a little ad-hoc (is class name always enough info, why are we 
sorting/uniquing here)

Consider returning vector and using a matcher to verify the things 
you care about (`kind(Decl::ClassTemplate)` or so?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

https://reviews.llvm.org/D148112

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet updated this revision to Diff 512775.
kadircet edited the summary of this revision.
kadircet added a comment.

- Use template instantion pattern helper instead of dealing with partial 
specializations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148112/new/

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 
+#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 +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 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 TargetDeclKinds;
   // Perform the walk, and capture the offsets of the referenced targets.
   std::unordered_map> 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  : 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 struct $explicit^Foo {};", "^Foo x;");
-  testWalk(R"cpp(
+}
+
+TEST(WalkAST, ClassTemplates) {
+  // Explicit instantiation and (partial) specialization references primary
+  // template.
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template struct ^Foo;"),
+  ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template<> struct ^Foo {};"),
+  ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template struct ^Foo {};"),
+  ElementsAre("ClassTemplate"));
+
+  // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+  testWalk("template struct $explicit^Foo {};", "^Foo x;"),
+  ElementsAre("CXXRecord"));
+  EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template<> struct $explicit^Foo {};)cpp",
-   "^Foo x;");
-  testWalk(R"cpp(
+   "^Foo x;"),
+  ElementsAre("ClassTemplateSpecialization"));
+  EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template struct $explicit^Foo { void x(); };)cpp",
-   "^Foo x;");
-  testWalk(R"cpp(
+   "^Foo x;"),
+  ElementsAre("ClassTemplatePartialSpecialization"));
+  EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template struct

[PATCH] D148112: [include-cleaner] Improve handling for templates

2023-04-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
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 
+#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 +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 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 TargetDeclKinds;
   // Perform the walk, and capture the offsets of the referenced targets.
   std::unordered_map> 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  : 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 struct $explicit^Foo {};", "^Foo x;");
-  testWalk(R"cpp(
+}
+
+TEST(WalkAST, ClassTemplates) {
+  // Explicit instantiation and (partial) specialization references primary
+  // template.
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template struct ^Foo;"),
+  ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template<> struct ^Foo {};"),
+  ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk("template struct $explicit^Foo{};",
+   "template struct ^Foo {};"),
+  ElementsAre("ClassTemplate"));
+
+  // Implicit instantiations references most relevant template.
+  EXPECT_THAT(
+  testWalk("template struct $explicit^Foo {};", "^Foo x;"),
+  ElementsAre("ClassTemplate"));
+  EXPECT_THAT(testWalk(R"cpp(
 template struct Foo {};
 template<> struct $explicit^Foo {};)cpp",
-   "^Foo x;");
-  testWalk(R"cpp(
+   "^Foo x;"),
+  ElementsAre("ClassTemplateSpecialization"));
+  EXPECT_THAT(testWalk(R"cpp(
 template