Index: clang-tools-extra/trunk/clangd/XRefs.cpp =================================================================== --- clang-tools-extra/trunk/clangd/XRefs.cpp +++ clang-tools-extra/trunk/clangd/XRefs.cpp @@ -876,21 +876,40 @@ return THI; } -static Optional getTypeAncestors(const CXXRecordDecl &CXXRD, - ASTContext &ASTCtx) { +using RecursionProtectionSet = llvm::SmallSet; + +static Optional +getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, + RecursionProtectionSet &RPSet) { Optional 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 ParentSym = - getTypeAncestors(*ParentDecl, ASTCtx)) { + getTypeAncestors(*ParentDecl, ASTCtx, RPSet)) { Result->parents->emplace_back(std::move(*ParentSym)); } } + if (Pattern) { + RPSet.erase(Pattern); + } + return Result; } @@ -933,10 +952,17 @@ ParentDecl = RT->getAsCXXRecordDecl(); } - // For now, do not handle dependent bases such as "Base". - // 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" by using the primary + // template. + if (const TemplateSpecializationType *TS = + Type->getAs()) { + TemplateName TN = TS->getTemplateName(); + if (TemplateDecl *TD = TN.getAsTemplateDecl()) { + ParentDecl = dyn_cast(TD->getTemplatedDecl()); + } + } + } if (ParentDecl) Result.push_back(ParentDecl); @@ -952,10 +978,13 @@ if (!CXXRD) return llvm::None; + RecursionProtectionSet RPSet; Optional Result = - getTypeAncestors(*CXXRD, AST.getASTContext()); + getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet); + // FIXME(nridge): Resolve type descendants if direction is Children or Both, // and ResolveLevels > 0. + return Result; } Index: clang-tools-extra/trunk/unittests/clangd/TypeHierarchyTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TypeHierarchyTests.cpp +++ clang-tools-extra/trunk/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 struct Parent {}; @@ -383,10 +381,10 @@ } } -TEST(TypeHierarchy, RecursiveHierarchy1) { +TEST(TypeHierarchy, RecursiveHierarchyUnbounded) { Annotations Source(R"cpp( template - struct S : S {}; + struct $SDef[[S]] : S {}; S^<0> s; )cpp"); @@ -399,62 +397,57 @@ ASSERT_TRUE(!AST.getDiagnostics().empty()); // Make sure getTypeHierarchy() doesn't get into an infinite recursion. + // FIXME(nridge): It would be preferable if the type hierarchy gave us type + // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than + // template names (e.g. "S"). llvm::Optional 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) { +TEST(TypeHierarchy, RecursiveHierarchyBounded) { Annotations Source(R"cpp( template - struct S : S {}; + struct $SDef[[S]] : S {}; template <> struct S<0>{}; - S^<2> s; - )cpp"); - - TestTU TU = TestTU::withCode(Source.code()); - auto AST = TU.build(); - - ASSERT_TRUE(AST.getDiagnostics().empty()); - - // Make sure getTypeHierarchy() doesn't get into an infinite recursion. - llvm::Optional Result = getTypeHierarchy( - AST, Source.points()[0], 0, TypeHierarchyDirection::Parents); - ASSERT_TRUE(bool(Result)); - EXPECT_THAT(*Result, - AllOf(WithName("S"), WithKind(SymbolKind::Struct), Parents())); -} - -TEST(TypeHierarchy, RecursiveHierarchy3) { - Annotations Source(R"cpp( - template - struct S : S {}; - - template <> - struct S<0>{}; + S$SRefConcrete^<2> s; template struct Foo { - S^ s; - }; - )cpp"); + S$SRefDependent^ s; + };)cpp"); TestTU TU = TestTU::withCode(Source.code()); auto AST = TU.build(); ASSERT_TRUE(AST.getDiagnostics().empty()); - // Make sure getTypeHierarchy() doesn't get into an infinite recursion. + // Make sure getTypeHierarchy() doesn't get into an infinite recursion + // for either a concrete starting point or a dependent starting point. llvm::Optional Result = getTypeHierarchy( - AST, Source.points()[0], 0, TypeHierarchyDirection::Parents); + AST, Source.point("SRefConcrete"), 0, TypeHierarchyDirection::Parents); + ASSERT_TRUE(bool(Result)); + EXPECT_THAT( + *Result, + AllOf(WithName("S"), WithKind(SymbolKind::Struct), + Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct), + SelectionRangeIs(Source.range("SDef")), Parents())))); + Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 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())))); } } // namespace