[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/index/MemIndex.cpp:35
 
 std::unique_ptr<SymbolIndex> MemIndex::build(SymbolSlab Slab) {
   auto Idx = llvm::make_unique<MemIndex>();
----------------
sammccall wrote:
> This is still implicitly creating an index with no occurrences. Did you mean to accept a SymbolOccurrencesSlab here?
This is intended, this function only cares about symbol, no occurrences.


================
Comment at: unittests/clangd/TestTU.h:41
 
+  static TestTU withAllCode(llvm::StringRef HeaderCode, llvm::StringRef Code,
+                            llvm::StringRef Filename = "") {
----------------
sammccall wrote:
> We've avoided adding this in the past because it's less readable. Please assign the fields separately instead.
I'm tempted to keep it here, while it's less readable, it does more things, and can make client code more readable (see newly-added tests), otherwise we have to repeat these statements in a few places.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279