nridge updated this revision to Diff 237572. nridge marked an inline comment as done. nridge added a comment.
Address nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71533/new/ https://reviews.llvm.org/D71533 Files: clang-tools-extra/clangd/XRefs.cpp clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -409,17 +409,21 @@ 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"). + // The parent is reported as "S" because "S<0>" is an invalid instantiation. + // We then iterate once more and find "S" again before detecting the + // recursion. 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(AllOf(WithName("S"), WithKind(SymbolKind::Struct), - SelectionRangeIs(Source.range("SDef")), Parents())))); + AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct), + Parents( + AllOf(WithName("S"), WithKind(SymbolKind::Struct), + SelectionRangeIs(Source.range("SDef")), + Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct), + SelectionRangeIs(Source.range("SDef")), + Parents())))))); } TEST(TypeHierarchy, RecursiveHierarchyBounded) { @@ -449,9 +453,12 @@ 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())))); + AllOf(WithName("S<2>"), WithKind(SymbolKind::Struct), + Parents(AllOf( + WithName("S<1>"), WithKind(SymbolKind::Struct), + SelectionRangeIs(Source.range("SDef")), + Parents(AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct), + Parents())))))); Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0, TypeHierarchyDirection::Parents); ASSERT_TRUE(bool(Result)); @@ -462,6 +469,83 @@ SelectionRangeIs(Source.range("SDef")), Parents())))); } +TEST(TypeHierarchy, DeriveFromImplicitSpec) { + Annotations Source(R"cpp( + template <typename T> + struct Parent {}; + + struct Child : Parent<int> {}; + + Parent<int> Fo^o; + )cpp"); + + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + ASSERT_TRUE(AST.getDiagnostics().empty()); + + llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy( + AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(), + testPath(TU.Filename)); + ASSERT_TRUE(bool(Result)); + EXPECT_THAT(*Result, + AllOf(WithName("Parent<int>"), WithKind(SymbolKind::Struct), + Children(AllOf(WithName("Child"), + WithKind(SymbolKind::Struct), Children())))); +} + +TEST(TypeHierarchy, DeriveFromPartialSpec) { + Annotations Source(R"cpp( + template <typename T> struct Parent {}; + template <typename T> struct Parent<T*> {}; + + struct Child : Parent<int*> {}; + + Parent<int> Fo^o; + )cpp"); + + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + ASSERT_TRUE(AST.getDiagnostics().empty()); + + llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy( + AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(), + testPath(TU.Filename)); + ASSERT_TRUE(bool(Result)); + EXPECT_THAT(*Result, AllOf(WithName("Parent<int>"), + WithKind(SymbolKind::Struct), Children())); +} + +TEST(TypeHierarchy, DeriveFromTemplate) { + Annotations Source(R"cpp( + template <typename T> + struct Parent {}; + + template <typename T> + struct Child : Parent<T> {}; + + Parent<int> Fo^o; + )cpp"); + + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + ASSERT_TRUE(AST.getDiagnostics().empty()); + + // FIXME: We'd like this to return the implicit specialization Child<int>, + // but currently libIndex does not expose relationships between + // implicit specializations. + llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy( + AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(), + testPath(TU.Filename)); + ASSERT_TRUE(bool(Result)); + EXPECT_THAT(*Result, + AllOf(WithName("Parent<int>"), WithKind(SymbolKind::Struct), + Children(AllOf(WithName("Child"), + WithKind(SymbolKind::Struct), Children())))); +} + SymbolID findSymbolIDByName(SymbolIndex *Index, llvm::StringRef Name, llvm::StringRef TemplateArgs = "") { SymbolID Result; Index: clang-tools-extra/clangd/XRefs.cpp =================================================================== --- clang-tools-extra/clangd/XRefs.cpp +++ clang-tools-extra/clangd/XRefs.cpp @@ -672,9 +672,18 @@ const SourceManager &SM = AST.getSourceManager(); SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation( getBeginningOfIdentifier(Pos, SM, AST.getLangOpts())); - DeclRelationSet Relations = - DeclRelation::TemplatePattern | DeclRelation::Underlying; - auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations); + unsigned Offset = + AST.getSourceManager().getDecomposedSpellingLoc(SourceLocationBeg).second; + SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset); + const SelectionTree::Node *N = Selection.commonAncestor(); + if (!N) + return nullptr; + + // Note: explicitReferenceTargets() will search for both template + // instantiations and template patterns, and prefer the former if available + // (generally, one will be available for non-dependent specializations of a + // class template). + auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Underlying); if (Decls.empty()) return nullptr; @@ -700,6 +709,13 @@ std::vector<const CXXRecordDecl *> typeParents(const CXXRecordDecl *CXXRD) { std::vector<const CXXRecordDecl *> Result; + // If this is an invalid instantiation, instantiation of the bases + // may not have succeeded, so fall back to the template pattern. + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(CXXRD)) { + if (CTSD->isInvalidDecl()) + CXXRD = CTSD->getSpecializedTemplate()->getTemplatedDecl(); + } + for (auto Base : CXXRD->bases()) { const CXXRecordDecl *ParentDecl = nullptr; @@ -754,6 +770,11 @@ Result->children.emplace(); if (Index) { + // The index does not store relationships between implicit + // specializations, so if we have one, use the template pattern instead. + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(CXXRD)) + CXXRD = CTSD->getTemplateInstantiationPattern(); + if (Optional<SymbolID> ID = getSymbolID(CXXRD)) fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits