[clangd] Handle templates more consistently in type hierarchy

[nridge]

Summary

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

[hokein]

thanks, the current behavior looks nicer than before.

IIUC, you're mostly fixing the issues mentioned in https://github.com/clangd/vscode-clangd/pull/44#issuecomment-669092168. If it just contains two fixes (this, and https://reviews.llvm.org/D86424), I think it might make sense to cherry-pick in release 11.

[nridge]

In D86861#2249490, @hokein wrote:

IIUC, you're mostly fixing the issues mentioned in https://github.com/clangd/vscode-clangd/pull/44#issuecomment-669092168. If it just contains two fixes (this, and https://reviews.llvm.org/D86424), I think it might make sense to cherry-pick in release 11.

There is also https://github.com/clangd/clangd/issues/510, and there may be other issues I haven't identified yet.

clang-tools-extra/clangd/XRefs.cpp

 llvm::Optional<TypeHierarchyItem>
 getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
                  TypeHierarchyDirection Direction, const SymbolIndex *Index,
                  PathRef TUPath) {
   const CXXRecordDecl *CXXRD = findRecordTypeAt(AST, Pos);
   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);
     }
   }
 
   return Result;
 }
 

clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

EXPECT_THAT(
                           SelectionRangeIs(Source.range("SDef")), Parents()))));
 }
 
 TEST(TypeHierarchy, DeriveFromImplicitSpec) {
   Annotations Source(R"cpp(
   template <typename T>
   struct Parent {};
 
-  struct Child : Parent<int> {};
+  struct Child1 : Parent<int> {};
+
+  struct Child2 : Parent<char> {};
 
   Parent<int> Fo^o;
   )cpp");
 
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
 
   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"),
+              AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+                    Children(AllOf(WithName("Child1"),
+                                   WithKind(SymbolKind::Struct), Children()),
+                             AllOf(WithName("Child2"),
                                    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();
 
   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()));
+  EXPECT_THAT(*Result, AllOf(WithName("Parent"), 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();
 
-  // 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()))));
 }
 
 TEST(TypeHierarchy, Preamble) {
   Annotations SourceAnnotations(R"cpp(
 struct Ch^ild : Parent {
   int b;