[PATCH] D86861: [clangd] Handle templates more consistently in type hierarchy

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: cfe-commits, danielkiss, usaxena95, kadircet, 
arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

If the tree includes types derived from all specializations of
a template, do not misleadingly label the root node with the
name of a single specialization.

Fixes https://github.com/clangd/clangd/issues/507


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86861

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
@@ -454,7 +454,9 @@
   template <typename T>
   struct Parent {};
 
-  struct Child : Parent<int> {};
+  struct Child1 : Parent<int> {};
+
+  struct Child2 : Parent<char> {};
 
   Parent<int> Fo^o;
   )cpp");
@@ -468,8 +470,10 @@
       testPath(TU.Filename));
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(*Result,
-              AllOf(WithName("Parent<int>"), WithKind(SymbolKind::Struct),
-                    Children(AllOf(WithName("Child"),
+              AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+                    Children(AllOf(WithName("Child1"),
+                                   WithKind(SymbolKind::Struct), Children()),
+                             AllOf(WithName("Child2"),
                                    WithKind(SymbolKind::Struct), Children()))));
 }
 
@@ -491,8 +495,8 @@
       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()));
+  EXPECT_THAT(*Result, AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+                             Children()));
 }
 
 TEST(TypeHierarchy, DeriveFromTemplate) {
@@ -510,15 +514,15 @@
   auto AST = TU.build();
   auto Index = TU.index();
 
-  // FIXME: We'd like this to return the implicit specialization Child<int>,
-  //        but currently libIndex does not expose relationships between
-  //        implicit specializations.
+  // FIXME: We'd like this to show the implicit specializations Parent<int>
+  //        and 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),
+              AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
                     Children(AllOf(WithName("Child"),
                                    WithKind(SymbolKind::Struct), Children()))));
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1479,30 +1479,39 @@
   if (!CXXRD)
     return llvm::None;
 
+  bool WantParents = Direction == TypeHierarchyDirection::Parents ||
+                     Direction == TypeHierarchyDirection::Both;
+  bool WantChildren = Direction == TypeHierarchyDirection::Children ||
+                      Direction == TypeHierarchyDirection::Both;
+
+  // If we're looking for children, we're doing the lookup in the index.
+  // The index does not store relationships between implicit
+  // specializations, so if we have one, use the template pattern instead.
+  // Note that this needs to be done before the declToTypeHierarchyItem(),
+  // otherwise the type hierarchy item would misleadingly contain the
+  // specialization parameters, while the children would involve classes
+  // that derive from other specializations of the template.
+  if (WantChildren) {
+    if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(CXXRD))
+      CXXRD = CTSD->getTemplateInstantiationPattern();
+  }
+
   Optional<TypeHierarchyItem> Result =
       declToTypeHierarchyItem(AST.getASTContext(), *CXXRD);
   if (!Result)
     return Result;
 
-  if (Direction == TypeHierarchyDirection::Parents ||
-      Direction == TypeHierarchyDirection::Both) {
+  if (WantParents) {
     Result->parents.emplace();
 
     RecursionProtectionSet RPSet;
     fillSuperTypes(*CXXRD, AST.getASTContext(), *Result->parents, RPSet);
   }
 
-  if ((Direction == TypeHierarchyDirection::Children ||
-       Direction == TypeHierarchyDirection::Both) &&
-      ResolveLevels > 0) {
+  if (WantChildren && ResolveLevels > 0) {
     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