[clangd] Fix typeHierarchy/supertypes when a base is an implicit instantiation

[ckandeler]

Summary

• superTypes() resolved each parent purely by looking up its SymbolID in the symbol index. When an immediate parent is an implicit template instantiation (e.g. a mixin template deriving from one of its own template parameters, as in template<typename Parent> struct Mixin : Parent, MixinBase {}; used as struct C : Mixin<A> {};), that instantiation is never indexed, so the lookup fails. Since it was the only immediate parent, the whole response degraded to no supertypes at all, hiding otherwise-indexed ancestors further up the chain.
• superTypes() now falls back to the already-computed nested parent data for any parent whose lookup fails, skipping over the unindexed link and surfacing its own parents directly instead.

Test plan

• Added Standard.SuperTypesSkipsUnindexedImplicitInstantiation in TypeHierarchyTests.cpp, reproducing the bug and verifying the fix.
• ninja check-clangd passes (1513 tests, 8 pre-existing unsupported, 0 failed).

Assisted-by: Claude

[llvmorg-github-actions]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: Christian Kandeler (ckandeler)

Changes

Summary

• superTypes() resolved each parent purely by looking up its SymbolID in the symbol index. When an immediate parent is an implicit template instantiation (e.g. a mixin template deriving from one of its own template parameters, as in template<typename Parent> struct Mixin : Parent, MixinBase {}; used as struct C : Mixin<A> {};), that instantiation is never indexed, so the lookup fails. Since it was the only immediate parent, the whole response degraded to no supertypes at all, hiding otherwise-indexed ancestors further up the chain.
• superTypes() now falls back to the already-computed nested parent data for any parent whose lookup fails, skipping over the unindexed link and surfacing its own parents directly instead.

Test plan

• Added Standard.SuperTypesSkipsUnindexedImplicitInstantiation in TypeHierarchyTests.cpp, reproducing the bug and verifying the fix.
• ninja check-clangd passes (1513 tests, 8 pre-existing unsupported, 0 failed).

Assisted-by: Claude

---

Full diff: https://github.com/llvm/llvm-project/pull/220240.diff

2 Files Affected:

• (modified) clang-tools-extra/clangd/XRefs.cpp (+26-8)
• (modified) clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp (+28)

diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 86528d806eab3..ca20eacd5eb82 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -2335,23 +2335,41 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
   return Results;
 }
 
-std::optional<std::vector<TypeHierarchyItem>>
-superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) {
-  if (!Index || !Item.data.parents)
-    return std::nullopt;
+// Resolves Parents against the index and appends the results to Results.
+// Parents that cannot be found in the index (e.g. because they are implicit
+// template instantiations, which clangd never indexes) are skipped over:
+// their own already-known parents are resolved instead, so a single
+// unindexed link in the chain doesn't hide everything above it.
+static void
+resolveParents(llvm::ArrayRef<TypeHierarchyItem::ResolveParams> Parents,
+               llvm::StringRef TUPath, const SymbolIndex &Index,
+               std::vector<TypeHierarchyItem> &Results) {
   LookupRequest Req;
   llvm::DenseMap<SymbolID, const TypeHierarchyItem::ResolveParams *> IDToData;
-  for (const auto &Parent : *Item.data.parents) {
+  for (const auto &Parent : Parents) {
     Req.IDs.insert(Parent.symbolID);
     IDToData[Parent.symbolID] = &Parent;
   }
-  std::vector<TypeHierarchyItem> Results;
-  Index->lookup(Req, [&Item, &Results, &IDToData](const Symbol &S) {
-    if (auto THI = symbolToTypeHierarchyItem(S, Item.uri.file())) {
+  llvm::DenseSet<SymbolID> Found;
+  Index.lookup(Req, [&](const Symbol &S) {
+    if (auto THI = symbolToTypeHierarchyItem(S, TUPath)) {
       THI->data = *IDToData.lookup(S.ID);
       Results.emplace_back(std::move(*THI));
+      Found.insert(S.ID);
     }
   });
+  for (const auto &Parent : Parents) {
+    if (!Found.contains(Parent.symbolID) && Parent.parents)
+      resolveParents(*Parent.parents, TUPath, Index, Results);
+  }
+}
+
+std::optional<std::vector<TypeHierarchyItem>>
+superTypes(const TypeHierarchyItem &Item, const SymbolIndex *Index) {
+  if (!Index || !Item.data.parents)
+    return std::nullopt;
+  std::vector<TypeHierarchyItem> Results;
+  resolveParents(*Item.data.parents, Item.uri.file(), *Index, Results);
   return Results.empty() ? std::nullopt
                          : std::make_optional(std::move(Results));
 }
diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
index 754063ede2724..e40e0ee11a8ae 100644
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -825,6 +825,34 @@ struct Chil^d : Parent {};
                   withSymbolTags(SymbolTag::Declaration, SymbolTag::Definition),
                   withResolveParents(Optional(IsEmpty()))))));
 }
+
+// A base class that is an implicit template instantiation
+// (like Mixin<RootBase> below) is never indexed, so
+// looking it up while resolving supertypes fails. superTypes() should not
+// let that hide everything above it; it should skip over the unresolvable
+// link and surface its own already-known parents instead.
+TEST(Standard, SuperTypesSkipsUnindexedImplicitInstantiation) {
+  Annotations Source(R"cpp(
+struct RootBase {};
+struct OtherBase {};
+template <typename T>
+struct Mixin : T, OtherBase {};
+struct Deri^ved : Mixin<RootBase> {};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+
+  auto Result = getTypeHierarchy(AST, Source.point(), /*ResolveLevels=*/1,
+                                 TypeHierarchyDirection::Children, Index.get(),
+                                 testPath(TU.Filename));
+  ASSERT_THAT(Result, SizeIs(1));
+  auto Parents = superTypes(Result.front(), Index.get());
+
+  EXPECT_THAT(Parents, Optional(UnorderedElementsAre(withName("RootBase"),
+                                                     withName("OtherBase"))));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

[HighCommander4]

Can you evaluate for overlap with #177273?

[timon-ul]

With just glancing at the code: What happens if you have the following code example:

struct RootBase {};
struct OtherBase {};

template <typename T>
struct Mixin : T {};

struct Unrelated : Mixin <OtherBase> {};
struct Deri^ved : Mixin <RootBase> {};

will it surface OtherBase in this case? I think it will if my understanding is still correct (and I don't think that is what we want).

I think the PR linked solves the issue without running into this problem, the main issue of it is that it needs a follow up to also be able to recognize the hierarchy in the direction wished for in this PR (currently it only is able to surface the implentations and not the base of a class).

Edit: Also if I remember correctly, said PR should surface Mixin<RootBase> too (with the location being the template) and if you expand it it will only give you Rootbase. And at least to me this behaviour overall sounds more desireable.