Skip to content

Commit af47038

Browse files
authored
[clangd] [C++20] [Modules] Support code complete for C++20 modules (#110083)
According to ChuanqiXu9/clangd-for-modules#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.
1 parent 0bc9834 commit af47038

File tree

4 files changed

+92
-1
lines changed

4 files changed

+92
-1
lines changed

clang-tools-extra/clangd/CodeComplete.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,9 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
14091409
Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
14101410
Clang->setCodeCompletionConsumer(Consumer.release());
14111411

1412+
if (Input.Preamble.RequiredModules)
1413+
Input.Preamble.RequiredModules->adjustHeaderSearchOptions(Clang->getHeaderSearchOpts());
1414+
14121415
SyntaxOnlyAction Action;
14131416
if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
14141417
log("BeginSourceFile() failed when running codeComplete for {0}",
@@ -2122,7 +2125,7 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
21222125
// When an is used, Sema is responsible for completing the main file,
21232126
// the index can provide results from the preamble.
21242127
// Tell Sema not to deserialize the preamble to look for results.
2125-
Result.LoadExternal = !Index;
2128+
Result.LoadExternal = ForceLoadPreamble || !Index;
21262129
Result.IncludeFixIts = IncludeFixIts;
21272130

21282131
return Result;

clang-tools-extra/clangd/CodeComplete.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ struct CodeCompleteOptions {
5252
/// For example, private members are usually inaccessible.
5353
bool IncludeIneligibleResults = false;
5454

55+
/// Force sema to load decls from preamble even if an index is provided.
56+
/// This is helpful for cases the index can't provide symbols, e.g. with
57+
/// experimental c++20 modules
58+
bool ForceLoadPreamble = false;
59+
5560
/// Combine overloads into a single completion item where possible.
5661
/// If none, the implementation may choose an appropriate behavior.
5762
/// (In practice, ClangdLSPServer enables bundling if the client claims

clang-tools-extra/clangd/tool/ClangdMain.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,9 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
919919
Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
920920
Opts.CodeComplete.RunParser = CodeCompletionParse;
921921
Opts.CodeComplete.RankingModel = RankingModel;
922+
// FIXME: If we're using C++20 modules, force the lookup process to load
923+
// external decls, since currently the index doesn't support C++20 modules.
924+
Opts.CodeComplete.ForceLoadPreamble = ExperimentalModulesSupport;
922925

923926
RealThreadsafeFS TFS;
924927
std::vector<std::unique_ptr<config::Provider>> ProviderStack;

clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,86 @@ import A;
402402
EXPECT_TRUE(D.isFromASTFile());
403403
}
404404

405+
// An end to end test for code complete in modules
406+
TEST_F(PrerequisiteModulesTests, CodeCompleteTest) {
407+
MockDirectoryCompilationDatabase CDB(TestDir, FS);
408+
409+
CDB.addFile("A.cppm", R"cpp(
410+
export module A;
411+
export void printA();
412+
)cpp");
413+
414+
llvm::StringLiteral UserContents = R"cpp(
415+
import A;
416+
void func() {
417+
print^
418+
}
419+
)cpp";
420+
421+
CDB.addFile("Use.cpp", UserContents);
422+
Annotations Test(UserContents);
423+
424+
ModulesBuilder Builder(CDB);
425+
426+
ParseInputs Use = getInputs("Use.cpp", CDB);
427+
Use.ModulesManager = &Builder;
428+
429+
std::unique_ptr<CompilerInvocation> CI =
430+
buildCompilerInvocation(Use, DiagConsumer);
431+
EXPECT_TRUE(CI);
432+
433+
auto Preamble =
434+
buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true,
435+
/*Callback=*/nullptr);
436+
EXPECT_TRUE(Preamble);
437+
EXPECT_TRUE(Preamble->RequiredModules);
438+
439+
auto Result = codeComplete(getFullPath("Use.cpp"), Test.point(),
440+
Preamble.get(), Use, {});
441+
EXPECT_FALSE(Result.Completions.empty());
442+
EXPECT_EQ(Result.Completions[0].Name, "printA");
443+
}
444+
445+
TEST_F(PrerequisiteModulesTests, SignatureHelpTest) {
446+
MockDirectoryCompilationDatabase CDB(TestDir, FS);
447+
448+
CDB.addFile("A.cppm", R"cpp(
449+
export module A;
450+
export void printA(int a);
451+
)cpp");
452+
453+
llvm::StringLiteral UserContents = R"cpp(
454+
import A;
455+
void func() {
456+
printA(^);
457+
}
458+
)cpp";
459+
460+
CDB.addFile("Use.cpp", UserContents);
461+
Annotations Test(UserContents);
462+
463+
ModulesBuilder Builder(CDB);
464+
465+
ParseInputs Use = getInputs("Use.cpp", CDB);
466+
Use.ModulesManager = &Builder;
467+
468+
std::unique_ptr<CompilerInvocation> CI =
469+
buildCompilerInvocation(Use, DiagConsumer);
470+
EXPECT_TRUE(CI);
471+
472+
auto Preamble =
473+
buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true,
474+
/*Callback=*/nullptr);
475+
EXPECT_TRUE(Preamble);
476+
EXPECT_TRUE(Preamble->RequiredModules);
477+
478+
auto Result = signatureHelp(getFullPath("Use.cpp"), Test.point(),
479+
*Preamble.get(), Use, MarkupKind::PlainText);
480+
EXPECT_FALSE(Result.signatures.empty());
481+
EXPECT_EQ(Result.signatures[0].label, "printA(int a) -> void");
482+
EXPECT_EQ(Result.signatures[0].parameters[0].labelString, "int a");
483+
}
484+
405485
} // namespace
406486
} // namespace clang::clangd
407487

0 commit comments

Comments
 (0)