[clang-tools-extra] r363506 - [clangd] Type hierarchy subtypes

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 15 19:31:37 PDT 2019


Author: nridge
Date: Sat Jun 15 19:31:37 2019
New Revision: 363506

URL: http://llvm.org/viewvc/llvm-project?rev=363506&view=rev
Log:
[clangd] Type hierarchy subtypes

Summary:
This builds on the relations support added in D59407, D62459, D62471,
and D62839 to implement type hierarchy subtypes.

Reviewers: kadircet

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, mgrang, arphaman,
jdoerfert, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D58880

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/FindSymbols.cpp
    clang-tools-extra/trunk/clangd/FindSymbols.h
    clang-tools-extra/trunk/clangd/XRefs.cpp
    clang-tools-extra/trunk/clangd/XRefs.h
    clang-tools-extra/trunk/clangd/test/type-hierarchy.test
    clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Sat Jun 15 19:31:37 2019
@@ -339,8 +339,7 @@ void ClangdServer::applyTweak(PathRef Fi
     // out a way to cache the format style.
     auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
                                        InpAST->Inputs.FS.get());
-    auto Formatted =
-        cleanupAndFormat(InpAST->Inputs.Contents, *Raw, Style);
+    auto Formatted = cleanupAndFormat(InpAST->Inputs.Contents, *Raw, Style);
     if (!Formatted)
       return CB(Formatted.takeError());
     return CB(replacementsToEdits(InpAST->Inputs.Contents, *Formatted));
@@ -487,11 +486,13 @@ void ClangdServer::findHover(PathRef Fil
 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) {
+  std::string FileCopy = File; // copy will be captured by the lambda
+  auto Action = [FileCopy, 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,
+                                FileCopy));
   };
 
   WorkScheduler.runWithAST("Type Hierarchy", File, Bind(Action, std::move(CB)));

Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Sat Jun 15 19:31:37 2019
@@ -39,6 +39,37 @@ struct ScoredSymbolGreater {
 
 } // namespace
 
+llvm::Expected<Location> symbolToLocation(const Symbol &Sym,
+                                          llvm::StringRef HintPath) {
+  // Prefer the definition over e.g. a function declaration in a header
+  auto &CD = Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration;
+  auto Uri = URI::parse(CD.FileURI);
+  if (!Uri) {
+    return llvm::make_error<llvm::StringError>(
+        formatv("Could not parse URI '{0}' for symbol '{1}'.", CD.FileURI,
+                Sym.Name),
+        llvm::inconvertibleErrorCode());
+  }
+  auto Path = URI::resolve(*Uri, HintPath);
+  if (!Path) {
+    return llvm::make_error<llvm::StringError>(
+        formatv("Could not resolve path for URI '{0}' for symbol '{1}'.",
+                Uri->toString(), Sym.Name),
+        llvm::inconvertibleErrorCode());
+  }
+  Location L;
+  // Use HintPath as TUPath since there is no TU associated with this
+  // request.
+  L.uri = URIForFile::canonicalize(*Path, HintPath);
+  Position Start, End;
+  Start.line = CD.Start.line();
+  Start.character = CD.Start.column();
+  End.line = CD.End.line();
+  End.character = CD.End.column();
+  L.range = {Start, End};
+  return L;
+}
+
 llvm::Expected<std::vector<SymbolInformation>>
 getWorkspaceSymbols(llvm::StringRef Query, int Limit,
                     const SymbolIndex *const Index, llvm::StringRef HintPath) {
@@ -65,37 +96,18 @@ getWorkspaceSymbols(llvm::StringRef Quer
       Req.Limit ? *Req.Limit : std::numeric_limits<size_t>::max());
   FuzzyMatcher Filter(Req.Query);
   Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
-    // Prefer the definition over e.g. a function declaration in a header
-    auto &CD = Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration;
-    auto Uri = URI::parse(CD.FileURI);
-    if (!Uri) {
-      log("Workspace symbol: Could not parse URI '{0}' for symbol '{1}'.",
-          CD.FileURI, Sym.Name);
-      return;
-    }
-    auto Path = URI::resolve(*Uri, HintPath);
-    if (!Path) {
-      log("Workspace symbol: Could not resolve path for URI '{0}' for symbol "
-          "'{1}'.",
-          Uri->toString(), Sym.Name);
+    auto Loc = symbolToLocation(Sym, HintPath);
+    if (!Loc) {
+      log("Workspace symbols: {0}", Loc.takeError());
       return;
     }
-    Location L;
-    // Use HintPath as TUPath since there is no TU associated with this
-    // request.
-    L.uri = URIForFile::canonicalize(*Path, HintPath);
-    Position Start, End;
-    Start.line = CD.Start.line();
-    Start.character = CD.Start.column();
-    End.line = CD.End.line();
-    End.character = CD.End.column();
-    L.range = {Start, End};
+
     SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
     std::string Scope = Sym.Scope;
     llvm::StringRef ScopeRef = Scope;
     ScopeRef.consume_back("::");
     SymbolInformation Info = {(Sym.Name + Sym.TemplateSpecializationArgs).str(),
-                              SK, L, ScopeRef};
+                              SK, *Loc, ScopeRef};
 
     SymbolQualitySignals Quality;
     Quality.merge(Sym);

Modified: clang-tools-extra/trunk/clangd/FindSymbols.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.h?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FindSymbols.h (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.h Sat Jun 15 19:31:37 2019
@@ -13,6 +13,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDSYMBOLS_H
 
 #include "Protocol.h"
+#include "index/Symbol.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
@@ -20,6 +21,10 @@ namespace clangd {
 class ParsedAST;
 class SymbolIndex;
 
+/// Helper function for deriving an LSP Location for a Symbol.
+llvm::Expected<Location> symbolToLocation(const Symbol &Sym,
+                                          llvm::StringRef HintPath);
+
 /// Searches for the symbols matching \p Query. The syntax of \p Query can be
 /// the non-qualified name or fully qualified of a symbol. For example,
 /// "vector" will match the symbol std::vector and "std::vector" would also

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Sat Jun 15 19:31:37 2019
@@ -1065,6 +1065,45 @@ declToTypeHierarchyItem(ASTContext &Ctx,
   return THI;
 }
 
+static Optional<TypeHierarchyItem>
+symbolToTypeHierarchyItem(const Symbol &S, const SymbolIndex *Index,
+                          PathRef TUPath) {
+  auto Loc = symbolToLocation(S, TUPath);
+  if (!Loc) {
+    log("Type hierarchy: {0}", Loc.takeError());
+    return llvm::None;
+  }
+  TypeHierarchyItem THI;
+  THI.name = S.Name;
+  THI.kind = indexSymbolKindToSymbolKind(S.SymInfo.Kind);
+  THI.deprecated = (S.Flags & Symbol::Deprecated);
+  THI.selectionRange = Loc->range;
+  // FIXME: Populate 'range' correctly
+  // (https://github.com/clangd/clangd/issues/59).
+  THI.range = THI.selectionRange;
+  THI.uri = Loc->uri;
+
+  return std::move(THI);
+}
+
+static void fillSubTypes(const SymbolID &ID,
+                         std::vector<TypeHierarchyItem> &SubTypes,
+                         const SymbolIndex *Index, int Levels, PathRef TUPath) {
+  RelationsRequest Req;
+  Req.Subjects.insert(ID);
+  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
+    if (Optional<TypeHierarchyItem> ChildSym =
+            symbolToTypeHierarchyItem(Object, Index, TUPath)) {
+      if (Levels > 1) {
+        ChildSym->children.emplace();
+        fillSubTypes(Object.ID, *ChildSym->children, Index, Levels - 1, TUPath);
+      }
+      SubTypes.emplace_back(std::move(*ChildSym));
+    }
+  });
+}
+
 using RecursionProtectionSet = llvm::SmallSet<const CXXRecordDecl *, 4>;
 
 static Optional<TypeHierarchyItem>
@@ -1160,7 +1199,8 @@ std::vector<const CXXRecordDecl *> typeP
 
 llvm::Optional<TypeHierarchyItem>
 getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
-                 TypeHierarchyDirection Direction) {
+                 TypeHierarchyDirection Direction, const SymbolIndex *Index,
+                 PathRef TUPath) {
   const CXXRecordDecl *CXXRD = findRecordTypeAt(AST, Pos);
   if (!CXXRD)
     return llvm::None;
@@ -1169,8 +1209,16 @@ getTypeHierarchy(ParsedAST &AST, Positio
   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, TUPath);
+    }
+  }
 
   return Result;
 }

Modified: clang-tools-extra/trunk/clangd/XRefs.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.h?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.h (original)
+++ clang-tools-extra/trunk/clangd/XRefs.h Sat Jun 15 19:31:37 2019
@@ -134,9 +134,9 @@ const CXXRecordDecl *findRecordTypeAt(Pa
 std::vector<const CXXRecordDecl *> typeParents(const CXXRecordDecl *CXXRD);
 
 /// Get type hierarchy information at \p Pos.
-llvm::Optional<TypeHierarchyItem>
-getTypeHierarchy(ParsedAST &AST, Position Pos, int Resolve,
-                 TypeHierarchyDirection Direction);
+llvm::Optional<TypeHierarchyItem> getTypeHierarchy(
+    ParsedAST &AST, Position Pos, int Resolve, TypeHierarchyDirection Direction,
+    const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{});
 
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/test/type-hierarchy.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/type-hierarchy.test?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/test/type-hierarchy.test (original)
+++ clang-tools-extra/trunk/clangd/test/type-hierarchy.test Sat Jun 15 19:31:37 2019
@@ -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": [

Modified: clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp?rev=363506&r1=363505&r2=363506&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp Sat Jun 15 19:31:37 2019
@@ -34,7 +34,7 @@ using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Pointee;
-using ::testing::UnorderedElementsAreArray;
+using ::testing::UnorderedElementsAre;
 
 // GMock helpers for matching TypeHierarchyItem.
 MATCHER_P(WithName, N, "") { return arg.name == N; }
@@ -450,6 +450,158 @@ TEST(TypeHierarchy, RecursiveHierarchyBo
                           SelectionRangeIs(Source.range("SDef")), Parents()))));
 }
 
+SymbolID findSymbolIDByName(SymbolIndex *Index, llvm::StringRef Name,
+                            llvm::StringRef TemplateArgs = "") {
+  SymbolID Result;
+  FuzzyFindRequest Request;
+  Request.Query = Name;
+  Request.AnyScope = true;
+  bool GotResult = false;
+  Index->fuzzyFind(Request, [&](const Symbol &S) {
+    if (TemplateArgs == S.TemplateSpecializationArgs) {
+      EXPECT_FALSE(GotResult);
+      Result = S.ID;
+      GotResult = true;
+    }
+  });
+  EXPECT_TRUE(GotResult);
+  return Result;
+}
+
+std::vector<SymbolID> collectSubtypes(SymbolID Subject, SymbolIndex *Index) {
+  std::vector<SymbolID> Result;
+  RelationsRequest Req;
+  Req.Subjects.insert(Subject);
+  Req.Predicate = index::SymbolRole::RelationBaseOf;
+  Index->relations(Req,
+                   [&Result](const SymbolID &Subject, const Symbol &Object) {
+                     Result.push_back(Object.ID);
+                   });
+  return Result;
+}
+
+TEST(Subtypes, SimpleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent {};
+struct Child1a : Parent {};
+struct Child1b : Parent {};
+struct Child2 : Child1a {};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent = findSymbolIDByName(Index.get(), "Parent");
+  SymbolID Child1a = findSymbolIDByName(Index.get(), "Child1a");
+  SymbolID Child1b = findSymbolIDByName(Index.get(), "Child1b");
+  SymbolID Child2 = findSymbolIDByName(Index.get(), "Child2");
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()),
+              UnorderedElementsAre(Child1a, Child1b));
+  EXPECT_THAT(collectSubtypes(Child1a, Index.get()), ElementsAre(Child2));
+}
+
+TEST(Subtypes, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {};
+struct Parent2 {};
+struct Parent3 : Parent2 {};
+struct Child : Parent1, Parent3 {};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto Index = TU.index();
+
+  SymbolID Parent1 = findSymbolIDByName(Index.get(), "Parent1");
+  SymbolID Parent2 = findSymbolIDByName(Index.get(), "Parent2");
+  SymbolID Parent3 = findSymbolIDByName(Index.get(), "Parent3");
+  SymbolID Child = findSymbolIDByName(Index.get(), "Child");
+
+  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(Index.get(), "Parent");
+  SymbolID Child = findSymbolIDByName(Index.get(), "Child");
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
+}
+
+TEST(Subtypes, 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(Index.get(), "Parent");
+  SymbolID ParentSpec = findSymbolIDByName(Index.get(), "Parent", "<int>");
+  SymbolID Child1 = findSymbolIDByName(Index.get(), "Child1");
+  SymbolID Child2 = findSymbolIDByName(Index.get(), "Child2");
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child1));
+  EXPECT_THAT(collectSubtypes(ParentSpec, Index.get()), ElementsAre(Child2));
+}
+
+TEST(Subtypes, 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(Index.get(), "Parent");
+  SymbolID ChildSpec = findSymbolIDByName(Index.get(), "Child", "<int>");
+
+  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(Index.get(), "Parent");
+  SymbolID Child = findSymbolIDByName(Index.get(), "Child");
+
+  EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list