[clang-tools-extra] 34f5774 - [include-cleaner] Improve handling for templates
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 12 08:41:27 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-04-12T17:36:05+02:00
New Revision: 34f5774920f54f81b3e3a361a7c7f21a61e39b39
URL: https://github.com/llvm/llvm-project/commit/34f5774920f54f81b3e3a361a7c7f21a61e39b39
DIFF: https://github.com/llvm/llvm-project/commit/34f5774920f54f81b3e3a361a7c7f21a61e39b39.diff
LOG: [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
Added:
Modified:
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index f702460ccbb22..25afdaa3a75b0 100644
--- 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 @@ 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());
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index dc649b1392294..8593d05a1b12d 100644
--- 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 <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
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 59e0ff117e88c..264bedf2e9ac7 100644
--- 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 <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 +41,9 @@ namespace {
// 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
diff erences, 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 {};
More information about the cfe-commits
mailing list