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

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/ClangdServer.h:208
                bool BuildDynamicSymbolIndex = false,
+               std::unique_ptr<SymbolIndex> StaticIdx = nullptr,
                llvm::Optional<StringRef> ResourceDir = llvm::None);
----------------
Please also add comment for this.


================
Comment at: clangd/ClangdServer.h:342
   std::unique_ptr<FileIndex> FileIdx;
+  /// If set, this provides static index for project-wide global symbols.
+  std::unique_ptr<SymbolIndex> StaticIdx;
----------------
... in addition to the `FileIdx` above?


================
Comment at: clangd/CodeComplete.cpp:571
+  // FIXME: find out a better way to show the index source.
+  Item.detail = llvm::Twine("[" + IndexSource + "]").str();
   return Item;
----------------
AFAIK, `Detail` is not visible in the completion item list unless you hover on an item. I'd suggest throwing a "[G]" prefix to labels of the global symbols. For completion `clang::^`, the list would look like:
```
clang::Local1
clang::Local2
[G]clang::Global1
[G]clang::Global2
```

WDYT?


================
Comment at: clangd/CodeComplete.cpp:651
                      std::move(PCHs));
-  if (Opts.Index && CompletedName.SSInfo) {
+  if (CompletedName.SSInfo && (Opts.Index || Opts.StaticIndex)) {
     if (!Results.items.empty())
----------------
We might not want to lose (non-index-based) AST symbols if we only have `StaticIndex`. Maybe only use static index augment the dynamic index?


================
Comment at: clangd/CodeComplete.h:72
+  /// Static index for project-wide global symbols. It is set by the clangd
+  /// client.
+  /// The static index is loaded and built only once when clangd is being
----------------
ioeric wrote:
> `clangd client` is a confusing term? Do you mean clangd library users? Also, I think this field is only set internally by clangd in this patch.
It's unclear whether this is set internally or expected to be set by users?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41668