[PATCH] D42049: [clangd] Merge results from static/dynamic index.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 15 01:21:28 PST 2018

hokein added inline comments.

Comment at: clangd/index/Merge.cpp:29
+     //    b) if it's in the dynamic slab, merge it and yield the result
+     //  3) now yield all the dynamic symbols we haven't processed.
+     bool More = false; // We'll be incomplete if either source was.
Maybe add another FIXME, saying filer out symbols in static index, but are removed in dynamic index?

Comment at: clangd/index/Merge.cpp:34
+         Dynamic->fuzzyFind(Ctx, Req, [&](const Symbol &S) { DynB.insert(S); });
+     SymbolSlab Dyn = std::move(DynB).build();
IIUC, `Dyn` is a local variable, which holds all the underlying data of `Symbol`, the `Symbol` returned in the callback of `fuzzyFind` would be invalid after the `fuzzyFind(..)` function call is finished.

Our previous assumption is that `Symbol` is valid as long as the `SymbolIndex` exists?

Comment at: clangd/index/Merge.cpp:75
+    if (S.Detail->Documentation == "")
+      S.Detail->Documentation = R.Detail->Documentation;
+  }
Don't we need ` CompletionDetail`?

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list