nridge updated this revision to Diff 234446.
nridge added a comment.

Address 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,60 @@
                           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, 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
@@ -674,13 +674,25 @@
   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::TemplateInstantiation |
+                              DeclRelation::TemplatePattern |
+                              DeclRelation::Underlying;
   auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
   if (Decls.empty())
     return nullptr;
 
-  const Decl *D = Decls[0];
+  const Decl *D = nullptr;
+  for (const Decl *Candidate : Decls) {
+    // Choose the first candidate that's a ClassTemplateSpecializationDecl,
+    // it there is one, otherwise just the find candidate in general.
+    if (!D || (isa<ClassTemplateSpecializationDecl>(Candidate) &&
+               !isa<ClassTemplateSpecializationDecl>(D))) {
+      D = Candidate;
+    }
+  }
 
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
     // If this is a variable, use the type of the variable.
@@ -756,6 +768,12 @@
     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