[clang-tools-extra] ca842c8 - [clangd] Handle templates more consistently in type hierarchy

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-09-01T19:18:37-04:00
New Revision: ca842c825a1caf10aacb1dc63664d565b1f2f4eb

URL: https://github.com/llvm/llvm-project/commit/ca842c825a1caf10aacb1dc63664d565b1f2f4eb
DIFF: https://github.com/llvm/llvm-project/commit/ca842c825a1caf10aacb1dc63664d565b1f2f4eb.diff

LOG: [clangd] Handle templates more consistently in type hierarchy

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

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/XRefs.cpp
    clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 031a9c7bf5da..67c857c378e1 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1479,30 +1479,39 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
   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);
     }

diff  --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
index 8e615e215fc5..9f021ade0278 100644
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -454,7 +454,9 @@ TEST(TypeHierarchy, DeriveFromImplicitSpec) {
   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 @@ TEST(TypeHierarchy, DeriveFromImplicitSpec) {
       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 @@ TEST(TypeHierarchy, DeriveFromPartialSpec) {
       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 @@ TEST(TypeHierarchy, DeriveFromTemplate) {
   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()))));
 }