[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)

Bevin Hansson via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 13 02:52:46 PST 2025


https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/120099

>From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Mon, 16 Dec 2024 16:40:06 +0100
Subject: [PATCH 1/5] [clangd] Augment code completion results with
 documentation from the index.

When looking up code completions from Sema, there is no associated
documentation. This is due to crash issues with stale preambles.
However, this also means that code completion results from other
than the main file do not have documentation in certain cases,
which is a bad user experience.

This patch performs a lookup into the index using the code
completion result declarations to find documentation, and
attaches it to the results.

This fixes clangd/clangd#2252 and clangd/clangd#564.
---
 clang-tools-extra/clangd/CodeComplete.cpp | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 2c2d5f0b5ac924..6916a32b7c2bac 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1863,14 +1863,41 @@ class CodeCompleteFlow {
     CodeCompleteResult Output;
 
     // Convert the results to final form, assembling the expensive strings.
+    // If necessary, search the index for documentation comments.
+    LookupRequest Req;
+    llvm::DenseMap<SymbolID, uint32_t> SymbolToCompletion;
     for (auto &C : Scored) {
       Output.Completions.push_back(toCodeCompletion(C.first));
       Output.Completions.back().Score = C.second;
       Output.Completions.back().CompletionTokenRange = ReplacedRange;
+      if (Opts.Index && !Output.Completions.back().Documentation) {
+        for (auto &Cand : C.first) {
+          if (Cand.SemaResult &&
+              Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) {
+            auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration());
+            if (!ID)
+              continue;
+            Req.IDs.insert(ID);
+            SymbolToCompletion[ID] = Output.Completions.size() - 1;
+          }
+        }
+      }
     }
     Output.HasMore = Incomplete;
     Output.Context = CCContextKind;
     Output.CompletionRange = ReplacedRange;
+
+    // Look up documentation from the index.
+    if (Opts.Index) {
+      Opts.Index->lookup(Req, [&](const Symbol &S) {
+        auto &C = Output.Completions[SymbolToCompletion.at(S.ID)];
+        if (S.Documentation.empty())
+          return;
+        C.Documentation.emplace();
+        parseDocumentation(S.Documentation, *C.Documentation);
+      });
+    }
+
     return Output;
   }
 

>From ace8121506029e9016df9ec3bffa9d465334e97d Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Tue, 17 Dec 2024 08:57:52 +0100
Subject: [PATCH 2/5] Add test.

---
 .../clangd/unittests/CodeCompleteTests.cpp    | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 3acacf496e77f9..827b64c882d583 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1101,6 +1101,43 @@ int x = foo^
       Contains(AllOf(named("foo"), doc("This comment should be retained!"))));
 }
 
+TEST(CompletionTest, CommentsOnMembers) {
+  MockFS FS;
+  MockCompilationDatabase CDB;
+
+  auto Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+
+  ClangdServer Server(CDB, FS, Opts);
+
+  FS.Files[testPath("foo.h")] = R"cpp(
+    struct alpha {
+      /// This is a member field.
+      int gamma;
+
+      /// This is a member function.
+      int delta();
+    };
+  )cpp";
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+#include "foo.h"
+alpha a;
+int x = a.^
+     )cpp");
+  runAddDocument(Server, File, Test.code());
+  auto CompletionList =
+      llvm::cantFail(runCodeComplete(Server, File, Test.point(), {}));
+
+  EXPECT_THAT(
+      CompletionList.Completions,
+      Contains(AllOf(named("gamma"), doc("This is a member field."))));
+  EXPECT_THAT(
+      CompletionList.Completions,
+      Contains(AllOf(named("delta"), doc("This is a member function."))));
+}
+
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");

>From 53978eccd6689486088aa5b76f39758357e339c4 Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Tue, 17 Dec 2024 09:33:03 +0100
Subject: [PATCH 3/5] Make clang-format happy.

---
 clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 827b64c882d583..607236c5b07925 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1130,9 +1130,8 @@ int x = a.^
   auto CompletionList =
       llvm::cantFail(runCodeComplete(Server, File, Test.point(), {}));
 
-  EXPECT_THAT(
-      CompletionList.Completions,
-      Contains(AllOf(named("gamma"), doc("This is a member field."))));
+  EXPECT_THAT(CompletionList.Completions,
+              Contains(AllOf(named("gamma"), doc("This is a member field."))));
   EXPECT_THAT(
       CompletionList.Completions,
       Contains(AllOf(named("delta"), doc("This is a member function."))));

>From 9b151814e8fff8d8d7dc7195445af6baa988b57f Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Mon, 13 Jan 2025 11:45:00 +0100
Subject: [PATCH 4/5] Address review comments.

---
 clang-tools-extra/clangd/CodeComplete.cpp     |  2 +-
 .../clangd/unittests/CodeCompleteTests.cpp    | 47 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 6916a32b7c2bac..7ec15b841286f3 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1890,9 +1890,9 @@ class CodeCompleteFlow {
     // Look up documentation from the index.
     if (Opts.Index) {
       Opts.Index->lookup(Req, [&](const Symbol &S) {
-        auto &C = Output.Completions[SymbolToCompletion.at(S.ID)];
         if (S.Documentation.empty())
           return;
+        auto &C = Output.Completions[SymbolToCompletion.at(S.ID)];
         C.Documentation.emplace();
         parseDocumentation(S.Documentation, *C.Documentation);
       });
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 607236c5b07925..32dc05f1e44b97 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1101,7 +1101,7 @@ int x = foo^
       Contains(AllOf(named("foo"), doc("This comment should be retained!"))));
 }
 
-TEST(CompletionTest, CommentsOnMembers) {
+TEST(CompletionTest, CommentsOnMembersFromHeader) {
   MockFS FS;
   MockCompilationDatabase CDB;
 
@@ -1137,6 +1137,51 @@ int x = a.^
       Contains(AllOf(named("delta"), doc("This is a member function."))));
 }
 
+TEST(CompletionTest, CommentsOnMembersFromHeaderOverloadBundling) {
+  using testing::AnyOf;
+  MockFS FS;
+  MockCompilationDatabase CDB;
+
+  auto Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+
+  ClangdServer Server(CDB, FS, Opts);
+
+  FS.Files[testPath("foo.h")] = R"cpp(
+    struct alpha {
+      /// bool overload.
+      int delta(bool b);
+
+      /// int overload.
+      int delta(int i);
+
+      void epsilon(long l);
+      
+      /// This one has a comment.
+      void epsilon(int i);
+    };
+  )cpp";
+
+  auto File = testPath("foo.cpp");
+  Annotations Test(R"cpp(
+#include "foo.h"
+alpha a;
+int x = a.^
+     )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions CCOpts;
+  CCOpts.BundleOverloads = true;
+  auto CompletionList =
+      llvm::cantFail(runCodeComplete(Server, File, Test.point(), CCOpts));
+
+  EXPECT_THAT(
+      CompletionList.Completions,
+      Contains(AllOf(named("epsilon"), doc("This one has a comment."))));
+  EXPECT_THAT(
+      CompletionList.Completions,
+      Contains(AllOf(named("delta"), AnyOf(doc("bool overload."), doc("int overload.")))));
+}
+
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");

>From d200ccfd79efa793f3eaf7562cd2e3842a6a443c Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Mon, 13 Jan 2025 11:52:28 +0100
Subject: [PATCH 5/5] Fix clang-format issues.

---
 clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 32dc05f1e44b97..f5b1adc9d352dc 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1177,9 +1177,9 @@ int x = a.^
   EXPECT_THAT(
       CompletionList.Completions,
       Contains(AllOf(named("epsilon"), doc("This one has a comment."))));
-  EXPECT_THAT(
-      CompletionList.Completions,
-      Contains(AllOf(named("delta"), AnyOf(doc("bool overload."), doc("int overload.")))));
+  EXPECT_THAT(CompletionList.Completions,
+              Contains(AllOf(named("delta"), AnyOf(doc("bool overload."),
+                                                   doc("int overload.")))));
 }
 
 TEST(CompletionTest, GlobalCompletionFiltering) {



More information about the cfe-commits mailing list