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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
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;
----------------
ioeric wrote:
> ... in addition to the `FileIdx` above?
Updated the comment.


================
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;
----------------
ioeric wrote:
> 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?
Yeah, this would make debugging easier. We could figure out a better shown way to the end users in the future.


================
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())
----------------
ioeric wrote:
> 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?
The `Opts.StaticIndex` check is not need here, removed it -- the StaticIndex is set when `enable-index-based-completion` is on. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41668