tom-anders added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1484
+ LookupRequest ContainerLookup;
+ llvm::DenseMap<SymbolID, size_t> RefIndexForContainer;
Results.HasMore |= Index->refs(Req, [&](const Ref &R) {
----------------
nridge wrote:
> We can have multiple references with the same container (e.g. multiple
> references in the same function), so I think we need `DenseMap<SymbolID,
> std::vector<size_t>>` here.
Good catch! Turns out this fixes the FIXME I added in
FindReferences.RefsToBaseMethod
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1501
}
+ SymbolID Container = R.Container;
+ ContainerLookup.IDs.insert(Container);
----------------
nridge wrote:
> For good measure, perhaps condition the container-related logic here on
> `AddContext`?
Good point, otherwise we're just wasting cycles and memory here if `AddContext`
is false anyway
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2238
for (const auto &R : Code.ranges())
ExpectedLocations.push_back(rangeIs(R));
EXPECT_THAT(findReferences(AST, Code.point(), 0).References,
----------------
nridge wrote:
> Did you mean to test the payload here?
Hmm I think I instead missed that this test does not use `checkFindRefs`.
Removed the payload here, this path is already covered by other tests anyway.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137894/new/
https://reviews.llvm.org/D137894
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
