[PATCH] D50385: [clangd] Collect symbol occurrences in SymbolCollector

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/index/Index.h:377
+   llvm::ArrayRef<SymbolOccurrence> find(const SymbolID &ID) const {
+     auto It = Occurrences.find(ID);
+     if (It == Occurrences.end())
----------------
sammccall wrote:
> return Occurrences.lookup(ID)?
The `DenseMap::lookup` returns a copy of `Value` (`vector`) which doesn't suit our use case :( -- we will return an `ArrayRef` which stores an reference of a local `vector` object.


================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:466
+      SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+      testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(
----------------
sammccall wrote:
> this is cute - if possible, consider adding a matcher factory function for readability here, so you can write `EXPECT_THAT(..., HaveRanges(Main.ranges("foo"))`
Wrapped this into `HaveRanges`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50385