[PATCH] D41281: [clangd] Index-based code completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Here are a few more comments.



================
Comment at: clangd/CodeComplete.cpp:847
+  if (Opts.Index && SCInfo.SSInfo) {
+    // FIXME: log warning with logger if sema code completion have collected
+    // results.
----------------
NIT: FIXME(ioeric)? 
Also, what's missing to resolve this FIXME in this commit?


================
Comment at: clangd/CodeComplete.cpp:329
+    Item.label = "::";
+  // FIXME: use more symbol information (e.g. documentation, label) to populate
+  // the completion item.
----------------
NIT: FIXME(ioeric)?


================
Comment at: clangd/CodeComplete.cpp:334
+  Item.insertTextFormat = InsertTextFormat::PlainText;
+  // FIXME: sort symbols appropriately.
+  Item.sortText = "";
----------------
NIT: FIXME(ioeric)?
Also, why not provide some `sortText` here? Like a qualified name. Because we're currently missing scores?


================
Comment at: clangd/CodeComplete.cpp:380
+  llvm::Optional<ScopeSpecifierInfo> SSInfo;
+  // FIXME: add more information for other completion cases that we care about.
+  // For example, non-qualified id completion.
----------------
NIT: FIXME(ioeric)?


================
Comment at: clangd/CodeComplete.h:64
+
+  // Populated internally by clangd, do not set.
+  /// If `Index` is set, it is used to augment the code completion
----------------
Given the comment, maybe we want to pass it as a separate parameter instead?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41281