sammccall created this revision.
sammccall added reviewers: hokein, nridge.
Herald added subscribers: usaxena95, kadircet, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.
There are several logical inconsistencies and traps in the current traversal.
This doesn't fix any of them, but
- describes the general expectations of how templates should be traversed
- (mostly) exhaustively records current behavior as a baseline for changes
- lists the apparent bugs in current behavior
See https://github.com/clangd/clangd/issues/1034 for an example of reasonable
code that falls into one of the traps, and some discussion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120498
Files:
clang/unittests/Tooling/CMakeLists.txt
clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
clang/unittests/Tooling/TestVisitor.h
Index: clang/unittests/Tooling/TestVisitor.h
===================================================================
--- clang/unittests/Tooling/TestVisitor.h
+++ clang/unittests/Tooling/TestVisitor.h
@@ -95,7 +95,7 @@
void HandleTranslationUnit(clang::ASTContext &Context) override {
Visitor->Context = &Context;
- Visitor->TraverseDecl(Context.getTranslationUnitDecl());
+ Visitor->getDerived().TraverseAST(Context);
}
private:
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
===================================================================
--- /dev/null
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
@@ -0,0 +1,423 @@
+//===- Templates.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This tests exactly which template-related declarations are traversed by
+// RecursiveASTVisitor in various configurations.
+//
+// shouldVisitTemplateInstantiations() controls traversal of AST nodes that
+// were created by instantiation, rather than by written code. This means:
+// - Primary template bodies are always traversed: they are written code
+// - User-defined specializations (partial or complete) are traversed too.
+// - Implicit instantiations are only traversed if shouldVisit() is true.
+// - Explicit instantiations are a tricky case:
+// - the "declaration part" (template args, function params etc) was written
+// and should always be traversed.
+// - the "definition part" (function body, class members, var initializer)
+// was instantiated and should only be traversed if shouldVisit() is true.
+// - the node itself (FunctionDecl, {Class,Var}TemplateSpecializationDecl)
+// must be visited. Overriders of VisitFunctionDecl() etc may need to
+// check getTemplateSpecializationKind() to know whether to use the body.
+//
+// Because RecursiveASTVisitor should also be able to traverse arbitrary
+// subtrees, the nodes representing explicit instantiations should behave in
+// a similar way when traversed directly. This is also tested.
+// (The historical behavior for functions with shouldVisit() off was:
+// explicit function instantiations were not visited at all, but if explicitly
+// passed to TraverseDecl() then the whole body was traversed).
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Specifiers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+
+namespace {
+
+// This RecursiveASTVisitor logs the traversal of template-related declarations,
+// as well as a few leaf nodes (so we can tell whether bodies are traversed).
+class TemplateVisitor : public TestVisitor<TemplateVisitor> {
+ using Base = TestVisitor;
+
+public:
+ bool Instantiations = false;
+ std::function<Decl *(ASTContext &)> ChooseTraversalRoot =
+ [](ASTContext &Ctx) { return Ctx.getTranslationUnitDecl(); };
+
+ bool shouldVisitTemplateInstantiations() { return Instantiations; }
+ bool shouldVisitImplicitCode() { return false; }
+
+ bool TraverseAST(ASTContext &Ctx) {
+ return TraverseDecl(ChooseTraversalRoot(Ctx));
+ }
+
+#define LOG_VISIT(NodeTy) \
+ bool Visit##NodeTy(NodeTy *N) { \
+ addLog("Visit" #NodeTy); \
+ addNodeDetail(N); \
+ return Base::Visit##NodeTy(N); \
+ }
+#define LOG_TRAVERSE(NodeTy) \
+ LOG_VISIT(NodeTy) \
+ bool Traverse##NodeTy(NodeTy *N) { \
+ addLog("Traverse" #NodeTy); \
+ ++Indent; \
+ bool Ret = Base::Traverse##NodeTy(N); \
+ --Indent; \
+ return Ret; \
+ }
+ LOG_TRAVERSE(FunctionTemplateDecl)
+ LOG_TRAVERSE(ClassTemplateDecl)
+ LOG_TRAVERSE(VarTemplateDecl)
+ LOG_TRAVERSE(FunctionDecl)
+ LOG_TRAVERSE(CXXRecordDecl)
+ LOG_TRAVERSE(VarDecl)
+ LOG_TRAVERSE(ClassTemplateSpecializationDecl)
+ LOG_TRAVERSE(ClassTemplatePartialSpecializationDecl)
+ LOG_TRAVERSE(VarTemplateSpecializationDecl)
+ LOG_TRAVERSE(VarTemplatePartialSpecializationDecl)
+ LOG_VISIT(CompoundStmt)
+ LOG_VISIT(FieldDecl)
+ LOG_VISIT(BinaryOperator)
+#undef LOG_TRAVERSE
+#undef LOG_VISIT
+
+ std::string takeLog() {
+ std::string Result = llvm::join(Log, "\n");
+ Log.clear();
+ return Result;
+ }
+
+private:
+ template <typename T> void addNodeDetail(T) {}
+ void addNodeDetail(FunctionDecl *D) {
+ addDetail(D->getNameAsString());
+ if (D->getDescribedFunctionTemplate())
+ addDetail("Generic");
+ addDetail(D->getTemplateSpecializationKind());
+ }
+ void addNodeDetail(CXXRecordDecl *D) {
+ addDetail(D->getNameAsString());
+ if (D->getDescribedClassTemplate())
+ addDetail("Generic");
+ addDetail(D->getTemplateSpecializationKind());
+ }
+ void addNodeDetail(VarDecl *D) {
+ addDetail(D->getNameAsString());
+ if (D->getDescribedVarTemplate())
+ addDetail("Generic");
+ addDetail(D->getTemplateSpecializationKind());
+ }
+
+ void addDetail(TemplateSpecializationKind TSK) {
+ switch (TSK) {
+ case TSK_Undeclared:
+ break;
+ case TSK_ImplicitInstantiation:
+ addDetail("ImplicitInstantiation");
+ break;
+ case TSK_ExplicitSpecialization:
+ addDetail("ExplicitSpecialization");
+ break;
+ case TSK_ExplicitInstantiationDeclaration:
+ addDetail("ExplicitInstantiationDeclaration");
+ break;
+ case TSK_ExplicitInstantiationDefinition:
+ addDetail("ExplicitInstantiationDefinition");
+ break;
+ }
+ }
+
+ void addLog(llvm::StringRef Message) {
+ Log.emplace_back(2 * Indent, ' ');
+ Log.back().append(Message.begin(), Message.end());
+ }
+ void addDetail(llvm::StringRef Detail) {
+ Log.back().push_back(' ');
+ Log.back().append(Detail.begin(), Detail.end());
+ }
+ unsigned Indent = 0;
+ std::vector<std::string> Log;
+};
+
+void checkLog(llvm::StringRef Description, TemplateVisitor &Visitor,
+ llvm::StringRef Code, llvm::StringRef Expected) {
+ SCOPED_TRACE(Code);
+ EXPECT_TRUE(Visitor.runOver(Code, TemplateVisitor::Lang_CXX14));
+ EXPECT_EQ(Expected.trim(), Visitor.takeLog()) << Description;
+}
+
+TEST(RecursiveASTVisitor, VisitsFunctionTemplates) {
+ TemplateVisitor Visitor;
+ const llvm::StringRef Code = R"cpp(
+ // Primary template. Should be traversed.
+ template <int> int foo() { return 0; }
+ // Explicit specialization. Should be traversed.
+ template <> int foo<1>() { return 1; }
+ // Implicit instantiation. Should be traversed if instantiations are on.
+ int x = foo<2>();
+ // Explicit instantiation declaration. Head should be traversed (no body).
+ extern template int foo<3>();
+ // Explicit instantiation. Head traversed, body only with instantiations on.
+ template int foo<4>();
+ )cpp";
+
+ // FIXME: explicit instantiation decl/definition are missing.
+ checkLog("Default mode, without instantiations", Visitor, Code, R"(
+TraverseFunctionTemplateDecl
+ VisitFunctionTemplateDecl
+ TraverseFunctionDecl
+ VisitFunctionDecl foo Generic
+ VisitCompoundStmt
+TraverseFunctionDecl
+ VisitFunctionDecl foo ExplicitSpecialization
+ VisitCompoundStmt
+TraverseVarDecl
+ VisitVarDecl x
+)");
+
+ Visitor.Instantiations = true;
+ checkLog("With instantiations", Visitor, Code, R"(
+TraverseFunctionTemplateDecl
+ VisitFunctionTemplateDecl
+ TraverseFunctionDecl
+ VisitFunctionDecl foo Generic
+ VisitCompoundStmt
+ TraverseFunctionDecl
+ VisitFunctionDecl foo ImplicitInstantiation
+ VisitCompoundStmt
+ TraverseFunctionDecl
+ VisitFunctionDecl foo ExplicitInstantiationDeclaration
+ TraverseFunctionDecl
+ VisitFunctionDecl foo ExplicitInstantiationDefinition
+ VisitCompoundStmt
+TraverseFunctionDecl
+ VisitFunctionDecl foo ExplicitSpecialization
+ VisitCompoundStmt
+TraverseVarDecl
+ VisitVarDecl x
+)");
+
+ Visitor.Instantiations = false;
+ Visitor.ChooseTraversalRoot = [](ASTContext &Ctx) -> Decl * {
+ for (auto *D : Ctx.getTranslationUnitDecl()->decls()) {
+ if (auto *FTD = llvm::dyn_cast<FunctionTemplateDecl>(D)) {
+ for (auto *Spec : FTD->specializations())
+ if (Spec->getTemplateSpecializationKind() ==
+ clang::TSK_ExplicitInstantiationDefinition)
+ return Spec;
+ }
+ }
+ ADD_FAILURE() << "Could not find an explicit instantiation!";
+ return Ctx.getTranslationUnitDecl();
+ };
+ // FIXME: we should not visit the body, as instantiation traversals are off.
+ checkLog("Explicitly traversing an explicit instantiation", Visitor, Code, R"(
+TraverseFunctionDecl
+ VisitFunctionDecl foo ExplicitInstantiationDefinition
+ VisitCompoundStmt
+)");
+}
+
+TEST(RecursiveASTVisitor, VisitsClassTemplates) {
+ TemplateVisitor Visitor;
+ const llvm::StringRef Code = R"cpp(
+ // Primary template. Should be traversed.
+ template <class> class foo { int a; };
+ // Explicit specialization. Should be traversed.
+ template <> class foo<int> { int b; };
+ // Partial specialization. Should be traversed.
+ template <class T> class foo<T*> { int c; };
+ // Implicit instantiation. Should be traversed if instantiations are on.
+ foo<char> x;
+ // Explicit instantiation declaration. Head should be traversed (no body).
+ extern template class foo<short>;
+ // Explicit instantiation. Head traversed, body only with instantiations on.
+ template class foo<double>;
+ )cpp";
+
+ checkLog("Default mode, without instantiations", Visitor, Code, R"(
+TraverseClassTemplateDecl
+ VisitClassTemplateDecl
+ TraverseCXXRecordDecl
+ VisitCXXRecordDecl foo Generic
+ VisitFieldDecl
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitSpecialization
+ VisitClassTemplateSpecializationDecl
+ VisitFieldDecl
+TraverseClassTemplatePartialSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitSpecialization
+ VisitClassTemplateSpecializationDecl
+ VisitClassTemplatePartialSpecializationDecl
+ VisitFieldDecl
+TraverseVarDecl
+ VisitVarDecl x
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitInstantiationDeclaration
+ VisitClassTemplateSpecializationDecl
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitInstantiationDefinition
+ VisitClassTemplateSpecializationDecl
+)");
+
+ Visitor.Instantiations = true;
+ // FIXME: why does the ExplicitInstantiationDeclaration have a body at all?
+ checkLog("With instantiations", Visitor, Code, R"(
+TraverseClassTemplateDecl
+ VisitClassTemplateDecl
+ TraverseCXXRecordDecl
+ VisitCXXRecordDecl foo Generic
+ VisitFieldDecl
+ TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ImplicitInstantiation
+ VisitClassTemplateSpecializationDecl
+ VisitFieldDecl
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitSpecialization
+ VisitClassTemplateSpecializationDecl
+ VisitFieldDecl
+TraverseClassTemplatePartialSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitSpecialization
+ VisitClassTemplateSpecializationDecl
+ VisitClassTemplatePartialSpecializationDecl
+ VisitFieldDecl
+TraverseVarDecl
+ VisitVarDecl x
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitInstantiationDeclaration
+ VisitClassTemplateSpecializationDecl
+ VisitFieldDecl
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitInstantiationDefinition
+ VisitClassTemplateSpecializationDecl
+ VisitFieldDecl
+)");
+
+ Visitor.Instantiations = false;
+ Visitor.ChooseTraversalRoot = [](ASTContext &Ctx) -> Decl * {
+ for (auto *D : Ctx.getTranslationUnitDecl()->decls()) {
+ if (auto *Spec = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D)) {
+ if (Spec->getTemplateSpecializationKind() ==
+ clang::TSK_ExplicitInstantiationDefinition)
+ return Spec;
+ }
+ }
+ ADD_FAILURE() << "Could not find an explicit instantiation!";
+ return Ctx.getTranslationUnitDecl();
+ };
+ checkLog("Explicitly traversing an explicit instantiation", Visitor, Code, R"(
+TraverseClassTemplateSpecializationDecl
+ VisitCXXRecordDecl foo ExplicitInstantiationDefinition
+ VisitClassTemplateSpecializationDecl
+)");
+}
+
+TEST(RecursiveASTVisitor, VisitsVarTemplates) {
+ TemplateVisitor Visitor;
+ const llvm::StringRef Code = R"cpp(
+ // Primary template. Should be traversed.
+ template <class T> float foo = 1 + 1;
+ // Explicit specialization. Should be traversed.
+ template <> float foo<int> = 2 + 2;
+ // Partial specialization. Should be traversed.
+ template <class T> float foo<T*> = 3 + 3;
+ // Implicit instantiation. Should be traversed if instantiations are on.
+ float x = foo<char>;
+ // Explicit instantiation declaration. Head should be traversed (no body).
+ extern template float foo<short>;
+ // Explicit instantiation. Head traversed, body only with instantiations on.
+ template float foo<double>;
+ )cpp";
+
+ // FIXME: explicit specialization's initializer should be traversed.
+ // FIXME: implicit instantiation should not be traversed.
+ checkLog("Default mode, without instantiations", Visitor, Code, R"(
+TraverseVarTemplateDecl
+ VisitVarTemplateDecl
+ TraverseVarDecl
+ VisitVarDecl foo Generic
+ VisitBinaryOperator
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitSpecialization
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplatePartialSpecializationDecl
+ VisitVarDecl foo ExplicitSpecialization
+ VisitVarTemplateSpecializationDecl
+ VisitVarTemplatePartialSpecializationDecl
+ VisitBinaryOperator
+TraverseVarDecl
+ VisitVarDecl x
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ImplicitInstantiation
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitInstantiationDeclaration
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitInstantiationDefinition
+ VisitVarTemplateSpecializationDecl
+)");
+
+ Visitor.Instantiations = true;
+ // FIXME: implicit instantiation is traversed twice.
+ // FIXME: implicit instantiation's initializer should be traversed.
+ // FIXME: explicit specialization's initializer should be traversed.
+ // FIXME: explicit instantiation's initializer should be traversed.
+ checkLog("With instantiations", Visitor, Code, R"(
+TraverseVarTemplateDecl
+ VisitVarTemplateDecl
+ TraverseVarDecl
+ VisitVarDecl foo Generic
+ VisitBinaryOperator
+ TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ImplicitInstantiation
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitSpecialization
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplatePartialSpecializationDecl
+ VisitVarDecl foo ExplicitSpecialization
+ VisitVarTemplateSpecializationDecl
+ VisitVarTemplatePartialSpecializationDecl
+ VisitBinaryOperator
+TraverseVarDecl
+ VisitVarDecl x
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ImplicitInstantiation
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitInstantiationDeclaration
+ VisitVarTemplateSpecializationDecl
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitInstantiationDefinition
+ VisitVarTemplateSpecializationDecl
+)");
+
+ Visitor.Instantiations = false;
+ Visitor.ChooseTraversalRoot = [](ASTContext &Ctx) -> Decl * {
+ for (auto *D : Ctx.getTranslationUnitDecl()->decls()) {
+ if (auto *Spec = llvm::dyn_cast<VarTemplateSpecializationDecl>(D)) {
+ if (Spec->getTemplateSpecializationKind() ==
+ clang::TSK_ExplicitInstantiationDefinition)
+ return Spec;
+ }
+ }
+ ADD_FAILURE() << "Could not find an explicit instantiation!";
+ return Ctx.getTranslationUnitDecl();
+ };
+ checkLog("Explicitly traversing an explicit instantiation", Visitor, Code, R"(
+TraverseVarTemplateSpecializationDecl
+ VisitVarDecl foo ExplicitInstantiationDefinition
+ VisitVarTemplateSpecializationDecl
+)");
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/CMakeLists.txt
===================================================================
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -50,6 +50,7 @@
RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
RecursiveASTVisitorTests/ParenExpr.cpp
RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
+ RecursiveASTVisitorTests/Templates.cpp
RecursiveASTVisitorTests/TraversalScope.cpp
RecursiveASTVisitorTestDeclVisitor.cpp
RecursiveASTVisitorTestPostOrderVisitor.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
