llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangd
Author: Christian Kandeler (ckandeler)
<details>
<summary>Changes</summary>
## 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
- [x] Added `Standard.SuperTypesSkipsUnindexedImplicitInstantiation` in
`TypeHierarchyTests.cpp`, reproducing the bug and verifying the fix.
- [x] `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
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/220240
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
