nridge updated this revision to Diff 204201.
nridge added a comment.
Rebase, add lit test, and post for review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58880/new/
https://reviews.llvm.org/D58880
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/test/type-hierarchy.test
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
@@ -450,6 +450,177 @@
SelectionRangeIs(Source.range("SDef")), Parents()))));
}
+SymbolID findSymbolIDByName(llvm::StringRef Name, SymbolIndex *Index) {
+ SymbolID Result;
+ FuzzyFindRequest Request;
+ Request.Query = Name;
+ Request.AnyScope = true;
+ Request.Limit = 1;
+ int ResultCount = 0;
+ Index->fuzzyFind(Request, [&](const Symbol &S) {
+ Result = S.ID;
+ ++ResultCount;
+ });
+ EXPECT_EQ(1, ResultCount);
+ return Result;
+}
+
+std::vector<SymbolID> collectSubtypes(SymbolID Type, SymbolIndex *Index) {
+ std::vector<SymbolID> Result;
+ Index->relations(
+ RelationsRequest{Type, index::SymbolRole::RelationBaseOf, llvm::None},
+ [&Result](const Symbol &S) { Result.push_back(S.ID); });
+ return Result;
+}
+
+TEST(Subtypes, SimpleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent {
+ int a;
+};
+
+struct Child1 : Parent {
+ int b;
+};
+
+struct Child2 : Child1 {
+ int c;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID Child1 = findSymbolIDByName("Child1", Index.get());
+ SymbolID Child2 = findSymbolIDByName("Child2", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child1));
+ EXPECT_THAT(collectSubtypes(Child1, Index.get()), ElementsAre(Child2));
+}
+
+TEST(Subtypes, MultipleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent1 {
+ int a;
+};
+
+struct Parent2 {
+ int b;
+};
+
+struct Parent3 : Parent2 {
+ int c;
+};
+
+struct Child : Parent1, Parent3 {
+ int d;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent1 = findSymbolIDByName("Parent1", Index.get());
+ SymbolID Parent2 = findSymbolIDByName("Parent2", Index.get());
+ SymbolID Parent3 = findSymbolIDByName("Parent3", Index.get());
+ SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent1, Index.get()), ElementsAre(Child));
+ EXPECT_THAT(collectSubtypes(Parent2, Index.get()), ElementsAre(Parent3));
+ EXPECT_THAT(collectSubtypes(Parent3, Index.get()), ElementsAre(Child));
+}
+
+TEST(Subtypes, ClassTemplate) {
+ Annotations Source(R"cpp(
+struct Parent {};
+
+template <typename T>
+struct Child : Parent {};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
+}
+
+// FIXME(nridge):
+// This test is failing because findSymbolIDByName("Parent<int>")
+// does not find the explicit specialization. Figure out why this
+// is the case even though we are now storing explicit specializations
+// and their template argument lists in the index.
+TEST(Subtypes, DISABLED_TemplateSpec1) {
+ Annotations Source(R"cpp(
+template <typename T>
+struct Parent {};
+
+template <>
+struct Parent<int> {};
+
+struct Child1 : Parent<float> {};
+
+struct Child2 : Parent<int> {};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID ParentSpec = findSymbolIDByName("Parent<int>", Index.get());
+ SymbolID Child1 = findSymbolIDByName("Child1", Index.get());
+ SymbolID Child2 = findSymbolIDByName("Child2", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child1));
+ EXPECT_THAT(collectSubtypes(ParentSpec, Index.get()), ElementsAre(Child2));
+}
+
+// FIXME(nridge):
+// This test is failing because findSymbolIDByName("Child<int>")
+// does not find the explicit specialization. Figure out why this
+// is the case even though we are now storing explicit specializations
+// and their template argument lists in the index.
+TEST(Subtypes, DISABLED_TemplateSpec2) {
+ Annotations Source(R"cpp(
+struct Parent {};
+
+template <typename T>
+struct Child {};
+
+template <>
+struct Child<int> : Parent {};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID ChildSpec = findSymbolIDByName("Child<int>", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(ChildSpec));
+}
+
+TEST(Subtypes, DependentBase) {
+ Annotations Source(R"cpp(
+template <typename T>
+struct Parent {};
+
+template <typename T>
+struct Child : Parent<T> {};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/test/type-hierarchy.test
===================================================================
--- clang-tools-extra/clangd/test/type-hierarchy.test
+++ clang-tools-extra/clangd/test/type-hierarchy.test
@@ -1,12 +1,39 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};\nstruct Child3 : Child2 {};"}}}
---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/typeHierarchy","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":11},"direction":1,"resolve":1}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/typeHierarchy","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":11},"direction":2,"resolve":1}}
# CHECK: "id": 1
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": {
+# CHECK-NEXT: "children": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "kind": 23,
+# CHECK-NEXT: "name": "Child3",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 13,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 7,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "selectionRange": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 13,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 7,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file:///clangd-test/main.cpp"
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
# CHECK-NEXT: "kind": 23,
# CHECK-NEXT: "name": "Child2",
# CHECK-NEXT: "parents": [
Index: clang-tools-extra/clangd/XRefs.h
===================================================================
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -136,7 +136,8 @@
/// Get type hierarchy information at \p Pos.
llvm::Optional<TypeHierarchyItem>
getTypeHierarchy(ParsedAST &AST, Position Pos, int Resolve,
- TypeHierarchyDirection Direction);
+ TypeHierarchyDirection Direction,
+ const SymbolIndex *Index = nullptr);
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1046,6 +1046,59 @@
return THI;
}
+static Optional<TypeHierarchyItem>
+symbolToTypeHierarchyItem(const Symbol &S, const SymbolIndex *Index) {
+ TypeHierarchyItem THI;
+ THI.name = S.Name;
+ THI.kind = indexSymbolKindToSymbolKind(S.SymInfo.Kind);
+ THI.deprecated = (S.Flags & Symbol::Deprecated);
+ Position Start, End;
+ auto &CD = S.Definition ? S.Definition : S.CanonicalDeclaration;
+ Start.line = CD.Start.line();
+ Start.character = CD.Start.column();
+ End.line = CD.End.line();
+ End.character = CD.End.column();
+ // TODO: How to get entire range like in declToTypeHierarchyItem()?
+ THI.range = {Start, End};
+ THI.selectionRange = {Start, End};
+ // TODO: Reuse code between here and getWorkspaceSymbols().
+ auto Uri = URI::parse(CD.FileURI);
+ if (!Uri) {
+ log("Type hierarchy: Could not parse URI '{0}' for symbol '{1}'.",
+ CD.FileURI, S.Name);
+ return llvm::None;
+ }
+ // TODO: Pass in ClangdServer::WorkspaceRoot as a HintPath.
+ StringRef HintPath;
+ auto Path = URI::resolve(*Uri, HintPath);
+ if (!Path) {
+ log("Type hierarchy: Could not resolve path for URI '{0}' for symbol "
+ "'{1}'.",
+ Uri->toString(), S.Name);
+ return llvm::None;
+ }
+ THI.uri = URIForFile::canonicalize(*Path, HintPath);
+
+ return std::move(THI);
+}
+
+static void fillSubTypes(const SymbolID &ID,
+ std::vector<TypeHierarchyItem> &SubTypes,
+ const SymbolIndex *Index, int Levels) {
+ Index->relations(
+ RelationsRequest{ID, index::SymbolRole::RelationBaseOf, llvm::None},
+ [Index, Levels, &SubTypes](const Symbol &Sub) {
+ if (Optional<TypeHierarchyItem> ChildSym =
+ symbolToTypeHierarchyItem(Sub, Index)) {
+ if (Levels > 1) {
+ ChildSym->children.emplace();
+ fillSubTypes(Sub.ID, *ChildSym->children, Index, Levels - 1);
+ }
+ SubTypes.emplace_back(std::move(*ChildSym));
+ }
+ });
+}
+
using RecursionProtectionSet = llvm::SmallSet<const CXXRecordDecl *, 4>;
static Optional<TypeHierarchyItem>
@@ -1141,7 +1194,7 @@
llvm::Optional<TypeHierarchyItem>
getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
- TypeHierarchyDirection Direction) {
+ TypeHierarchyDirection Direction, const SymbolIndex *Index) {
const CXXRecordDecl *CXXRD = findRecordTypeAt(AST, Pos);
if (!CXXRD)
return llvm::None;
@@ -1150,8 +1203,17 @@
Optional<TypeHierarchyItem> Result =
getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
- // FIXME(nridge): Resolve type descendants if direction is Children or Both,
- // and ResolveLevels > 0.
+ if ((Direction == TypeHierarchyDirection::Children ||
+ Direction == TypeHierarchyDirection::Both) &&
+ ResolveLevels > 0) {
+ Result->children.emplace();
+
+ if (Index) {
+ if (Optional<SymbolID> ID = getSymbolID(CXXRD)) {
+ fillSubTypes(*ID, *Result->children, Index, ResolveLevels);
+ }
+ }
+ }
return Result;
}
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -480,11 +480,11 @@
void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve,
TypeHierarchyDirection Direction,
Callback<Optional<TypeHierarchyItem>> CB) {
- auto Action = [Pos, Resolve, Direction](decltype(CB) CB,
- Expected<InputsAndAST> InpAST) {
+ auto Action = [Pos, Resolve, Direction, this](decltype(CB) CB,
+ Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
- CB(clangd::getTypeHierarchy(InpAST->AST, Pos, Resolve, Direction));
+ CB(clangd::getTypeHierarchy(InpAST->AST, Pos, Resolve, Direction, Index));
};
WorkScheduler.runWithAST("Type Hierarchy", File, Bind(Action, std::move(CB)));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
