[clangd] Type hierarchy: don't resolve parents if the client only asked for children

[nridge]

Summary

Also reorganize the code for computing supertypes to make it more symmetric to subtypes.

[nridge]

This is an issue I noticed while writing the unit test for D64308.

[kadircet]

clang-tools-extra/clangd/XRefs.cpp

1238 ↗

(On Diff #209407)

having a function named getTypeAncestors with a parameter ResolveParents doesn't make much sense. maybe move the check to caller side and simply don't call it if we are not interested in parents?

I think it makes sense to make this part also similar to subtypes:

• let's generate the item in here with Optional<TypeHierarchyItem> Result = declToTypeHierarchyItem(ASTCtx, CXXRD);, which is used by both parents and children.
• bail out if we couldn't get the item.
• fill in parents if need be
• fill in children if need be

WDYT?

[nridge]

Address review comment

[nridge]

clang-tools-extra/clangd/XRefs.cpp

1238 ↗

(On Diff #209407)

Agreed, this makes the code cleaner.

[nridge]

(Changed reviewer to Kadir as he reviewed the previous version.)

[kadircet]

LGTM, thanks!

clang-tools-extra/trunk/clangd/XRefs.cpp

if (Optional<TypeHierarchyItem> ChildSym =
       }
       SubTypes.emplace_back(std::move(*ChildSym));
     }
   });
 }
 
 using RecursionProtectionSet = llvm::SmallSet<const CXXRecordDecl *, 4>;
 
-static Optional<TypeHierarchyItem>
-getTypeAncestors(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
+static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
+                           std::vector<TypeHierarchyItem> &SuperTypes,
                            RecursionProtectionSet &RPSet) {
-  Optional<TypeHierarchyItem> Result = declToTypeHierarchyItem(ASTCtx, CXXRD);
-  if (!Result)
-    return Result;
-
-  Result->parents.emplace();
-
   // typeParents() will replace dependent template specializations
   // with their class template, so to avoid infinite recursion for
   // certain types of hierarchies, keep the templates encountered
   // along the parent chain in a set, and stop the recursion if one
   // starts to repeat.
   auto *Pattern = CXXRD.getDescribedTemplate() ? &CXXRD : nullptr;
   if (Pattern) {
     if (!RPSet.insert(Pattern).second) {
-      return Result;
+      return;
     }
   }
 
   for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) {
     if (Optional<TypeHierarchyItem> ParentSym =
-            getTypeAncestors(*ParentDecl, ASTCtx, RPSet)) {
-      Result->parents->emplace_back(std::move(*ParentSym));
+            declToTypeHierarchyItem(ASTCtx, *ParentDecl)) {
+      ParentSym->parents.emplace();
+      fillSuperTypes(*ParentDecl, ASTCtx, *ParentSym->parents, RPSet);
+      SuperTypes.emplace_back(std::move(*ParentSym));
     }
   }
 
   if (Pattern) {
     RPSet.erase(Pattern);
   }
-
-  return Result;
 }
 
 const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) {
   SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
       AST, Pos, AST.getSourceManager().getMainFileID());
   IdentifiedSymbol Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
   if (Symbols.Decls.empty())
     return nullptr;

 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;
 
-  RecursionProtectionSet RPSet;
   Optional<TypeHierarchyItem> Result =
-      getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
+      declToTypeHierarchyItem(AST.getASTContext(), *CXXRD);
   if (!Result)
     return Result;
 
+  if (Direction == TypeHierarchyDirection::Parents ||
+      Direction == TypeHierarchyDirection::Both) {
+    Result->parents.emplace();
+
+    RecursionProtectionSet RPSet;
+    fillSuperTypes(*CXXRD, AST.getASTContext(), *Result->parents, RPSet);
+  }
+
   if ((Direction == TypeHierarchyDirection::Children ||
        Direction == TypeHierarchyDirection::Both) &&
       ResolveLevels > 0) {
     Result->children.emplace();
 
     if (Index) {
       if (Optional<SymbolID> ID = getSymbolID(CXXRD))
         fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath);

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

)cpp");
   auto Index = TU.index();
 
   llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
       AST, Source.point(), /*ResolveLevels=*/1,
       TypeHierarchyDirection::Children, Index.get(), testPath(TU.Filename));
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
       *Result,
-      AllOf(WithName("Parent"), WithKind(SymbolKind::Struct), Parents(),
+      AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+            ParentsNotResolved(),
             Children(AllOf(WithName("Child1"), WithKind(SymbolKind::Struct),
                            ParentsNotResolved(), ChildrenNotResolved()))));
 
   resolveTypeHierarchy((*Result->children)[0], /*ResolveLevels=*/1,
                        TypeHierarchyDirection::Children, Index.get());
 
   EXPECT_THAT(
       (*Result->children)[0],