[PATCH] D50337: [clangd] DexIndex implementation prototype

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

LG. Last few nits and then good to go.



================
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:97
+    }
+    TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+
----------------
Check `!TrigramIterators.empty()`?


================
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:128
+      const llvm::Optional<float> Score = Filter.match(Sym->Name);
+      if (!Score.hasValue())
+        continue;
----------------
nit: `if (!Score)`


================
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:149
+                      llvm::function_ref<void(const Symbol &)> Callback) const {
+  for (const auto &ID : Req.IDs) {
+    auto I = LookupTable.find(ID);
----------------
This also needs lock.


================
Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:23
+
+Symbol symbol(llvm::StringRef QName);
+
----------------
Add comment about what `SymbolID` is?


================
Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:25
+
+struct SlabAndPointers {
+  SymbolSlab Slab;
----------------
Please add documentation.


================
Comment at: clang-tools-extra/unittests/clangd/TestIndex.h:46
+
+std::vector<std::string> match(const SymbolIndex &I,
+                               const FuzzyFindRequest &Req,
----------------
Please add documentation.


https://reviews.llvm.org/D50337