[PATCH] D41668: [clangd] Add static index for the global code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/ClangdLSPServer.h:40
+                  bool BuildDynamicSymbolIndex,
+                  std::unique_ptr<SymbolIndex> StaticIdx);
 
----------------
is there a reason ClangdServer should own this? I can imagine this complicating life for embedders. (vs a raw pointer)


================
Comment at: clangd/CodeComplete.cpp:554
+                                   const SpecifiedScope &SSInfo,
+                                   llvm::StringRef IndexSource = "") {
   CompletionItem Item;
----------------
ioeric wrote:
> Maybe `IndexSourceLabel`?
Actually, can we give this a more generic name like DebuggingLabel?
That will make things less confusing as we store slightly different info here over time (e.g. this text will definitely change when merging happens). It also indicates that this isn't functionally important.

If you don't mind, I'd also change the value so you just pass "G" and it gets formatted into "[G] " by this function. That means we can easily format it differently (e.g. hide it from the UI actually include it in the JSON as an extension field) in a way that makes sense.


================
Comment at: clangd/CodeComplete.cpp:591
 
-  Items->isIncomplete = !Index.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
-    Items->items.push_back(indexCompletionItem(Sym, Filter, SSInfo));
-  });
+  // FIXME: figure out a good algorithm to merge symbols from dynamic index and
+  // static index.
----------------
nit: this comment implies that we should do a two-way merge here between static/dynamic index. I don't think that's the case.
We're also going want to merge with AST based results, and probably want to do that *before* generating CompletionItems, because scoring should take signals from all sources into account.

I'd suggest reverting this function to use just one index, and calling it twice.
Then this comment about merging would live near the call site, and cover all three sources - it'd suggest the fix where we should be applying it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41668