[clangd] Add Documentations for member completions.

[hokein]

Summary

We are missing docs for class member completion -- we don't store
comments in the preamble, so Sema doesn't return any docs. To get docs
for class members, we rely on the index (mostly dynamic index).

Tried it on llvm, didn't get noticeble delay for the completion.

[hokein]

Add a comment.

[ilya-biryukov]

clangd/CodeComplete.cpp

1373–1377

naming NIT: the loop var should be I

1375

Since we already have two variables, maybe deconstruct the pair into one of them instead of keeping a variable for a pair too? I.e.

auto &Bundle = Top[I].first;
auto &Score = Top[I].second;

should make the code a little simpler

1382

NIT: use early exits (see the LLVM Style guide) to reduce nesting here.

if (size() != 1)
  continue;
auto *SemaR = ;
if (!SemaR)
  continue;
// ...

[hokein]

Address comments.

[sammccall]

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?

clangd/CodeComplete.cpp

1369

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.

1398

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.

clangd/CodeComplete.cpp

CodeCompleteResult runWithSema() {
     auto IndexResults = (Opts.Index && allowIndex(Recorder->CCContext))
                             ? queryIndex()
                             : SymbolSlab();
     trace::Span Tracer("Populate CodeCompleteResult");
     // Merge Sema and Index results, score them, and pick the winners.
     auto Top = mergeResults(Recorder->Results, IndexResults);
     CodeCompleteResult Output;
 
+    // Keys are indices into Output vector.
+    llvm::DenseMap<size_t, SymbolID> OutputIndex;
+    LookupRequest DocIndexRequest;
     // Convert the results to final form, assembling the expensive strings.
-    for (auto &C : Top) {
-      Output.Completions.push_back(toCodeCompletion(C.first));
-      Output.Completions.back().Score = C.second;
+    for (size_t I = 0; I < Top.size(); ++I) {
+      const auto& Bundle = Top[I].first;
+      const auto& Scope = Top[I].second;
+      Output.Completions.push_back(toCodeCompletion(Bundle));
+      Output.Completions.back().Score = Scope;
       Output.Completions.back().CompletionTokenRange = TextEditRange;
+
+      if (Opts.IncludeComments &&
+          Output.Completions.back().Documentation.empty()) {
+        if (Bundle.size() != 1)
+          continue;
+        const auto *SemaR = Bundle.front().SemaResult;
+        if (!SemaR)
+          continue;
+        if (auto ID =
+                getSymbolID(*SemaR, Recorder->CCSema->getSourceManager())) {
+          OutputIndex[I] = *ID;
+          DocIndexRequest.IDs.insert(*ID);
+        }
+      }
+    }
+    // Sema doesn't load docs from the preamble, so we get the docs from the
+    // index and assemble them for the final results.
+    if (!DocIndexRequest.IDs.empty() && Opts.Index) {
+      llvm::DenseMap<SymbolID, std::string> FetchedDocs;
+      Opts.Index->lookup(DocIndexRequest, [&](const Symbol &S) {
+        if (!S.Documentation.empty())
+          FetchedDocs[S.ID] = S.Documentation;
+      });
+      for (auto IndexAndID : OutputIndex) {
+        auto FetchDocsIt = FetchedDocs.find(IndexAndID.second);
+        if (FetchDocsIt != FetchedDocs.end())
+          Output.Completions[IndexAndID.first].Documentation =
+              FetchDocsIt->second;
+      }
     }
     Output.HasMore = Incomplete;
     Output.Context = Recorder->CCContext.getKind();
 
     return Output;
   }
 
   SymbolSlab queryIndex() {

unittests/clangd/CodeCompleteTests.cpp

TEST(CompletionTest, DynamicIndexMultiFile) {
   EXPECT_THAT(Results.Completions, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.Completions, Has("foo", CompletionItemKind::Function));
   EXPECT_THAT(Results.Completions, Has("XXX", CompletionItemKind::Class));
   EXPECT_THAT(Results.Completions,
               Contains((Named("fooooo"), Kind(CompletionItemKind::Function),
                         Doc("Doooc"), ReturnType("void"))));
 }
 
+TEST(CompletionTest, DocumentationFromIndex) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  FS.Files[testPath("foo.h")] = R"cpp(
+      class Foo {
+      public:
+        // Doc for foo
+        int foo();
+
+        // Doc for foo int
+        int foo2(int);
+        // Doc for foo bool
+        int foo2(bool);
+      };
+  )cpp";
+
+  auto File = testPath("bar.cpp");
+  Annotations Test(R"cpp(
+      #include "foo.h"
+      void test() {
+        Foo f;
+        f.^
+      }
+  )cpp");
+  auto Opts = ClangdServer::optsForTest();
+  {
+    // Run code completion without index, verify that we don't get any docs from
+    // Sema.
+    Opts.BuildDynamicSymbolIndex = false;
+    ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+    runAddDocument(Server, File, Test.code());
+    auto Results = cantFail(runCodeComplete(Server, File, Test.point(), {}));
+    EXPECT_THAT(
+        Results.Completions,
+        Contains((Named("foo"), Kind(CompletionItemKind::Method), Doc(""))));
+    EXPECT_THAT(
+        Results.Completions,
+        Contains((Named("foo2"), Kind(CompletionItemKind::Method), Doc(""))));
+  }
+  {
+    Opts.BuildDynamicSymbolIndex = true;
+    ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+    runAddDocument(Server, File, Test.code());
+    clangd::CodeCompleteOptions CCOpts;
+    CCOpts.BundleOverloads = true;
+    auto Results =
+        cantFail(runCodeComplete(Server, File, Test.point(), CCOpts));
+    EXPECT_THAT(Results.Completions,
+                Contains((Named("foo"), Kind(CompletionItemKind::Method),
+                          Doc("Doc for foo"))));
+    // No doc for overload bundle.
+    EXPECT_THAT(
+        Results.Completions,
+        Contains((Named("foo2"), Kind(CompletionItemKind::Method), Doc(""))));
+  }
+}
+
 TEST(CompletionTest, Documentation) {
   auto Results = completions(
       R"cpp(
       // Non-doxygen comment.
       int foo();
       /// Doxygen comment.
       /// \param int a
       int bar(int a);