[PATCH] D56492: [clangd] Add Documentations for member completions.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I think the design should be more thoroughly considered here.

- what are the latency consequences of the extra index lookup in different scenarios?
- how does this compare to doing it at LSP resolve time instead?
- if we're going to do the extra lookup, can we make use of ranking signals from the index too?



================
Comment at: clangd/CodeComplete.cpp:1369
 
+    // Keys are indices into Output vector.
+    llvm::DenseMap<size_t, SymbolID> OutputIndex;
----------------
I don't think we can inline this much logic into `runWithSema()` for each feature we add - need to find a clearer way to structure the code.


================
Comment at: clangd/CodeComplete.cpp:1398
+      llvm::DenseMap<SymbolID, std::string> FetchedDocs;
+      Opts.Index->lookup(DocIndexRequest, [&](const Symbol &S) {
+        if (!S.Documentation.empty())
----------------
If we're going to query the index again here, it seems we should do it earlier so we can use the results for ranking.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56492/new/

https://reviews.llvm.org/D56492