nridge updated this revision to Diff 234784.
nridge marked 4 inline comments as done.
nridge added a comment.

Address most review comments


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,13 @@
   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").
+  // Here, we actually don't get any parents, because the unbounded hierarchy
+  // causes instantiation of the base specifier to fail.
   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()))));
+  EXPECT_THAT(*Result,
+              AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct), Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +445,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 +461,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
@@ -137,7 +137,7 @@
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
   std::vector<const Decl *> Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
-    auto Decls = targetDecl(N->ASTNode, Relations);
+    auto Decls = explicitReferenceTargets(N->ASTNode, Relations);
     Result.assign(Decls.begin(), Decls.end());
   }
   return Result;
@@ -229,8 +229,7 @@
   }
 
   // Emit all symbol locations (declaration or definition) from AST.
-  DeclRelationSet Relations =
-      DeclRelation::TemplatePattern | DeclRelation::Alias;
+  DeclRelationSet Relations = DeclRelation::Alias;
   for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
     const Decl *Def = getDefinition(D);
     const Decl *Preferred = Def ? Def : D;
@@ -371,8 +370,7 @@
                                                       Position Pos) {
   const SourceManager &SM = AST.getSourceManager();
   // FIXME: show references to macro within file?
-  DeclRelationSet Relations =
-      DeclRelation::TemplatePattern | DeclRelation::Alias;
+  DeclRelationSet Relations = DeclRelation::Alias;
   auto References = findRefs(
       getDeclAtPosition(AST,
                         SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
@@ -417,8 +415,7 @@
   // TODO: should we handle macros, too?
   // We also show references to the targets of using-decls, so we include
   // DeclRelation::Underlying.
-  DeclRelationSet Relations = DeclRelation::TemplatePattern |
-                              DeclRelation::Alias | DeclRelation::Underlying;
+  DeclRelationSet Relations = DeclRelation::Alias | DeclRelation::Underlying;
   auto Decls = getDeclAtPosition(AST, Loc, Relations);
 
   // We traverse the AST to find references in the main file.
@@ -484,8 +481,7 @@
 
   // We also want the targets of using-decls, so we include
   // DeclRelation::Underlying.
-  DeclRelationSet Relations = DeclRelation::TemplatePattern |
-                              DeclRelation::Alias | DeclRelation::Underlying;
+  DeclRelationSet Relations = DeclRelation::Alias | DeclRelation::Underlying;
   for (const Decl *D : getDeclAtPosition(AST, Loc, Relations)) {
     SymbolDetails NewSymbol;
     if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
@@ -654,8 +650,10 @@
   const SourceManager &SM = AST.getSourceManager();
   SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
       getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
-  DeclRelationSet Relations =
-      DeclRelation::TemplatePattern | DeclRelation::Underlying;
+  // Search for both template instantiations and template patterns.
+  // We prefer the former, if available (generally, one will be
+  // available for non-dependent specializations of a class template).
+  DeclRelationSet Relations = DeclRelation::Underlying;
   auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
   if (Decls.empty())
     return nullptr;
@@ -736,6 +734,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

Reply via email to