[clang-tools-extra] [clangd] [C++20] [Modules] Support code complete for C++20 modules (PR #110083)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: Chuanqi Xu (ChuanqiXu9)

<details>
<summary>Changes</summary>

According to https://github.com/ChuanqiXu9/clangd-for-modules/issues/9, I surprisingly found the support for C++20 modules doesn't support code completion well.

After debugging, I found there are problems:
(1) We forgot to call `adjustHeaderSearchOptions` in code complete. This may be an easy oversight.
(2) In `CodeCompleteOptions::getClangCompleteOpts`, we may set `LoadExternal` as false when index is available. But we have support modules with index. So it is conflicting. Given modules are opt in now, I think it makes sense to to set LoadExternal as true when modules are enabled.

This is a small fix and I wish it can land faster.

---
Full diff: https://github.com/llvm/llvm-project/pull/110083.diff


3 Files Affected:

- (modified) clang-tools-extra/clangd/CodeComplete.cpp (+13-5) 
- (modified) clang-tools-extra/clangd/CodeComplete.h (+1-1) 
- (modified) clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp (+40) 


``````````diff
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 89eee392837af4..f6f0d9a1be3c37 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -892,8 +892,9 @@ static bool isExcludedMember(const NamedDecl &D) {
 // within the callback.
 struct CompletionRecorder : public CodeCompleteConsumer {
   CompletionRecorder(const CodeCompleteOptions &Opts,
+                     bool ForceLoadExternal,
                      llvm::unique_function<void()> ResultsCallback)
-      : CodeCompleteConsumer(Opts.getClangCompleteOpts()),
+      : CodeCompleteConsumer(Opts.getClangCompleteOpts(ForceLoadExternal)),
         CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
         CCAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
         CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {
@@ -1409,6 +1410,9 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
+  if (Input.Preamble.RequiredModules)
+    Input.Preamble.RequiredModules->adjustHeaderSearchOptions(Clang->getHeaderSearchOpts());
+
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     log("BeginSourceFile() failed when running codeComplete for {0}",
@@ -1629,11 +1633,15 @@ class CodeCompleteFlow {
       SpecFuzzyFind->Result = startAsyncFuzzyFind(*Opts.Index, *SpecReq);
     }
 
+    // FIXME: If we're using C++20 modules, force the lookup process to load external decls,
+    // since currently the index doesn't support C++20 modules.
+    bool ForceLoadExternal = (bool)SemaCCInput.Preamble.RequiredModules;
+
     // We run Sema code completion first. It builds an AST and calculates:
     //   - completion results based on the AST.
     //   - partial identifier and context. We need these for the index query.
     CodeCompleteResult Output;
-    auto RecorderOwner = std::make_unique<CompletionRecorder>(Opts, [&]() {
+    auto RecorderOwner = std::make_unique<CompletionRecorder>(Opts, ForceLoadExternal, [&]() {
       assert(Recorder && "Recorder is not set");
       CCContextKind = Recorder->CCContext.getKind();
       IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
@@ -1691,7 +1699,7 @@ class CodeCompleteFlow {
 
     Recorder = RecorderOwner.get();
 
-    semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
+    semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(ForceLoadExternal),
                      SemaCCInput, &Includes);
     logResults(Output, Tracer);
     return Output;
@@ -2108,7 +2116,7 @@ class CodeCompleteFlow {
 
 } // namespace
 
-clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
+clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts(bool ForceLoadExternal) const {
   clang::CodeCompleteOptions Result;
   Result.IncludeCodePatterns = EnableSnippets;
   Result.IncludeMacros = true;
@@ -2122,7 +2130,7 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
   // When an is used, Sema is responsible for completing the main file,
   // the index can provide results from the preamble.
   // Tell Sema not to deserialize the preamble to look for results.
-  Result.LoadExternal = !Index;
+  Result.LoadExternal = ForceLoadExternal || !Index;
   Result.IncludeFixIts = IncludeFixIts;
 
   return Result;
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
index a7c1ae95dcbf49..336e84f0a7724c 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -41,7 +41,7 @@ struct CodeCompletion;
 
 struct CodeCompleteOptions {
   /// Returns options that can be passed to clang's completion engine.
-  clang::CodeCompleteOptions getClangCompleteOpts() const;
+  clang::CodeCompleteOptions getClangCompleteOpts(bool ForceLoadExternal) const;
 
   /// When true, completion items will contain expandable code snippets in
   /// completion (e.g.  `return ${1:expression}` or `foo(${1:int a}, ${2:int
diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
index 7bbb95c8b8d67f..f1cdf9e899f4d8 100644
--- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
+++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
@@ -402,6 +402,46 @@ import A;
   EXPECT_TRUE(D.isFromASTFile());
 }
 
+// An end to end test for code complete in modules
+TEST_F(PrerequisiteModulesTests, CodeCompleteTest) {
+  MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+  CDB.addFile("A.cppm", R"cpp(
+export module A;
+export void printA();
+  )cpp");
+
+  llvm::StringLiteral UserContents = R"cpp(
+import A;
+void func() {
+  print^
+}
+)cpp";
+
+  CDB.addFile("Use.cpp", UserContents);
+  Annotations Test(UserContents);
+
+  ModulesBuilder Builder(CDB);
+
+  ParseInputs Use = getInputs("Use.cpp", CDB);
+  Use.ModulesManager = &Builder;
+
+  std::unique_ptr<CompilerInvocation> CI =
+      buildCompilerInvocation(Use, DiagConsumer);
+  EXPECT_TRUE(CI);
+
+  auto Preamble =
+      buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true,
+                    /*Callback=*/nullptr);
+  EXPECT_TRUE(Preamble);
+  EXPECT_TRUE(Preamble->RequiredModules);
+  
+  auto Result = codeComplete(getFullPath("Use.cpp"), Test.point(),
+                             Preamble.get(), Use, {});
+  EXPECT_FALSE(Result.Completions.empty());
+  EXPECT_EQ(Result.Completions[0].Name, "printA");
+}
+
 } // namespace
 } // namespace clang::clangd
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/110083