sammccall created this revision.
sammccall added reviewers: hokein, davidxl.
Herald added subscribers: wenlei, usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
This is consistent with the idea of shouldVisitTemplateInstantiations():
when false, we should traverse the code the user wrote but not the code
clang synthesized. Therefore we should be able to traverse the
signature of the function without traversing its body.
This is also consistent with how class and variable templates behave.
Note that currently RAV does not traverse *any* part of the these when
shouldVisitTemplateInstantiations() is false (which is another bug).
So currently this change is only observable when the user passes the
instantiated FunctionDecl to TraverseDecl themselves.
This is what the caller in CodeGenPGO does, and it wants to traverse the
instantiated body. I believe not traversing instantiations in general in
this context is an oversight.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120504
Files:
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
clang/include/clang/AST/RecursiveASTVisitor.h
clang/lib/CodeGen/CodeGenPGO.cpp
clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
===================================================================
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp
@@ -218,11 +218,9 @@
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
)");
}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -154,6 +154,8 @@
struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
using Base = RecursiveASTVisitor<MapRegionCounters>;
+ bool shouldVisitTemplateInstantiations() { return true; }
+
/// The next counter value to assign.
unsigned NextCounter;
/// The function hash.
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2064,6 +2064,7 @@
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
TRY_TO(TraverseDeclarationNameInfo(D->getNameInfo()));
+ bool BodyWasInstantiated = false;
// If we're an explicit template specialization, iterate over the
// template args that were explicitly specified. If we were doing
// this in typing order, we'd do it between the return type and
@@ -2071,8 +2072,7 @@
// above, so we have to choose one side. I've decided to do before.
if (const FunctionTemplateSpecializationInfo *FTSI =
D->getTemplateSpecializationInfo()) {
- if (FTSI->getTemplateSpecializationKind() != TSK_Undeclared &&
- FTSI->getTemplateSpecializationKind() != TSK_ImplicitInstantiation) {
+ if (FTSI->isExplicitInstantiationOrSpecialization()) {
// A specialization might not have explicit template arguments if it has
// a templated return type and concrete arguments.
if (const ASTTemplateArgumentListInfo *TALI =
@@ -2081,6 +2081,8 @@
TALI->NumTemplateArgs));
}
}
+ BodyWasInstantiated =
+ isTemplateInstantiation(FTSI->getTemplateSpecializationKind());
}
// Visit the function type itself, which can be either
@@ -2104,16 +2106,11 @@
TRY_TO(TraverseStmt(TrailingRequiresClause));
}
- if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
- // Constructor initializers.
- for (auto *I : Ctor->inits()) {
- if (I->isWritten() || getDerived().shouldVisitImplicitCode())
- TRY_TO(TraverseConstructorInitializer(I));
- }
- }
-
bool VisitBody =
D->isThisDeclarationADefinition() &&
+ // Don't visit the function body if it was instantiated, not written.
+ (!BodyWasInstantiated ||
+ getDerived().shouldVisitTemplateInstantiations()) &&
// Don't visit the function body if the function definition is generated
// by clang.
(!D->isDefaulted() || getDerived().shouldVisitImplicitCode());
@@ -2128,6 +2125,14 @@
}
if (VisitBody) {
+ if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
+ // Constructor initializers.
+ for (auto *I : Ctor->inits()) {
+ if (I->isWritten() || getDerived().shouldVisitImplicitCode())
+ TRY_TO(TraverseConstructorInitializer(I));
+ }
+ }
+
TRY_TO(TraverseStmt(D->getBody()));
// Body may contain using declarations whose shadows are parented to the
// FunctionDecl itself.
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -473,6 +473,18 @@
ExpectedHint{": int", "waldo"});
}
+TEST(TypeHints, FunctionTemplateExplicitInstantiation) {
+ // We used to erroneously emit bad type hints in the template for every
+ // explicit specialization: https://github.com/clangd/clangd/issues/1034
+ assertTypeHints(R"cpp(
+ template <class T> void foo() {
+ auto x = T{};
+ }
+ template void foo<char>();
+ template void foo<float>();
+ )cpp");
+}
+
TEST(TypeHints, Decorations) {
assertTypeHints(R"cpp(
int x = 42;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
