nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
Herald added a project: clang.
Dependent bases are handled heuristically, by replacing them with the class
template that they are a specialization of, where possible. Care is taken
to avoid infinite recursion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D59756
Files:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
Index: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
+++ clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp
@@ -291,9 +291,7 @@
EXPECT_THAT(typeParents(ChildSpec), ElementsAre(Parent));
}
-// This is disabled for now, because support for dependent bases
-// requires additional measures to avoid infinite recursion.
-TEST(DISABLED_TypeParents, DependentBase) {
+TEST(TypeParents, DependentBase) {
Annotations Source(R"cpp(
template <typename T>
struct Parent {};
@@ -386,7 +384,7 @@
TEST(TypeHierarchy, RecursiveHierarchy1) {
Annotations Source(R"cpp(
template <int N>
- struct S : S<N + 1> {};
+ struct $SDef[[S]] : S<N + 1> {};
S^<0> s;
)cpp");
@@ -402,14 +400,17 @@
llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
ASSERT_TRUE(bool(Result));
- EXPECT_THAT(*Result,
- AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+ EXPECT_THAT(
+ *Result,
+ AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ SelectionRangeIs(Source.range("SDef")), Parents()))));
}
TEST(TypeHierarchy, RecursiveHierarchy2) {
Annotations Source(R"cpp(
template <int N>
- struct S : S<N - 1> {};
+ struct $SDef[[S]] : S<N - 1> {};
template <>
struct S<0>{};
@@ -426,14 +427,17 @@
llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
ASSERT_TRUE(bool(Result));
- EXPECT_THAT(*Result,
- AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+ EXPECT_THAT(
+ *Result,
+ AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ SelectionRangeIs(Source.range("SDef")), Parents()))));
}
TEST(TypeHierarchy, RecursiveHierarchy3) {
Annotations Source(R"cpp(
template <int N>
- struct S : S<N - 1> {};
+ struct $SDef[[S]] : S<N - 1> {};
template <>
struct S<0>{};
@@ -453,8 +457,11 @@
llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
ASSERT_TRUE(bool(Result));
- EXPECT_THAT(*Result,
- AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents()));
+ EXPECT_THAT(
+ *Result,
+ AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+ SelectionRangeIs(Source.range("SDef")), Parents()))));
}
SymbolID findSymbolIDByName(llvm::StringRef Name, SymbolIndex *Index) {
@@ -599,8 +606,7 @@
EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(ChildSpec));
}
-// Disabled for the same reason as TypeParents.DependentBase.
-TEST(DISABLED_Subtypes, DependentBase) {
+TEST(Subtypes, DependentBase) {
Annotations Source(R"cpp(
template <typename T>
struct Parent {};
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -935,20 +935,40 @@
});
}
-static Optional<TypeHierarchyItem> getTypeAncestors(const CXXRecordDecl &CXXRD,
- ASTContext &ASTCtx) {
+using RecursionProtectionSet = llvm::SmallSet<const CXXRecordDecl *, 4>;
+
+static Optional<TypeHierarchyItem>
+getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
+ RecursionProtectionSet &RPSet) {
Optional<TypeHierarchyItem> Result = declToTypeHierarchyItem(ASTCtx, CXXRD);
if (!Result)
return Result;
Result->parents.emplace();
+ // typeParents() will replace dependent template specializations
+ // with their class template, so to avoid infinite recursion for
+ // certain types of hierarchies, keep the templates encountered
+ // along the parent chain in a set, and stop the recursion if one
+ // starts to repeat.
+ auto *Pattern = CXXRD.getDescribedTemplate() ? &CXXRD : nullptr;
+ if (Pattern) {
+ if (!RPSet.insert(Pattern).second) {
+ return Result;
+ }
+ }
+
for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) {
if (Optional<TypeHierarchyItem> ParentSym =
- getTypeAncestors(*ParentDecl, ASTCtx)) {
+ getTypeAncestors(*ParentDecl, ASTCtx, RPSet)) {
Result->parents->emplace_back(std::move(*ParentSym));
}
}
+
+ if (Pattern) {
+ RPSet.erase(Pattern);
+ }
+
return Result;
}
@@ -991,10 +1011,17 @@
ParentDecl = RT->getAsCXXRecordDecl();
}
- // For now, do not handle dependent bases such as "Base<T>".
- // We would like to handle them by heuristically choosing the
- // primary template declaration, but we need to take care to
- // avoid infinite recursion.
+ if (!ParentDecl) {
+ // Handle a dependent base such as "Base<T>" by using the primary
+ // template.
+ if (const TemplateSpecializationType *TS =
+ Type->getAs<TemplateSpecializationType>()) {
+ TemplateName TN = TS->getTemplateName();
+ if (TemplateDecl *TD = TN.getAsTemplateDecl()) {
+ ParentDecl = dyn_cast<CXXRecordDecl>(TD->getTemplatedDecl());
+ }
+ }
+ }
if (ParentDecl)
Result.push_back(ParentDecl);
@@ -1010,8 +1037,9 @@
if (!CXXRD)
return llvm::None;
+ RecursionProtectionSet RPSet;
Optional<TypeHierarchyItem> Result =
- getTypeAncestors(*CXXRD, AST.getASTContext());
+ getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
if ((Direction == TypeHierarchyDirection::Children ||
Direction == TypeHierarchyDirection::Both) &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
