Skip to content

Commit

Permalink
[clangd] [C++20] [Modules] Support code complete for C++20 modules (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
ChuanqiXu9 authored Sep 30, 2024
1 parent 0bc9834 commit af47038
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 1 deletion.
5 changes: 4 additions & 1 deletion clang-tools-extra/clangd/CodeComplete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,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}",
Expand Down Expand Up @@ -2122,7 +2125,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 = ForceLoadPreamble || !Index;
Result.IncludeFixIts = IncludeFixIts;

return Result;
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/clangd/CodeComplete.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ struct CodeCompleteOptions {
/// For example, private members are usually inaccessible.
bool IncludeIneligibleResults = false;

/// Force sema to load decls from preamble even if an index is provided.
/// This is helpful for cases the index can't provide symbols, e.g. with
/// experimental c++20 modules
bool ForceLoadPreamble = false;

/// Combine overloads into a single completion item where possible.
/// If none, the implementation may choose an appropriate behavior.
/// (In practice, ClangdLSPServer enables bundling if the client claims
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/clangd/tool/ClangdMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,9 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
Opts.CodeComplete.RunParser = CodeCompletionParse;
Opts.CodeComplete.RankingModel = RankingModel;
// 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.
Opts.CodeComplete.ForceLoadPreamble = ExperimentalModulesSupport;

RealThreadsafeFS TFS;
std::vector<std::unique_ptr<config::Provider>> ProviderStack;
Expand Down
80 changes: 80 additions & 0 deletions clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,86 @@ 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");
}

TEST_F(PrerequisiteModulesTests, SignatureHelpTest) {
MockDirectoryCompilationDatabase CDB(TestDir, FS);

CDB.addFile("A.cppm", R"cpp(
export module A;
export void printA(int a);
)cpp");

llvm::StringLiteral UserContents = R"cpp(
import A;
void func() {
printA(^);
}
)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 = signatureHelp(getFullPath("Use.cpp"), Test.point(),
*Preamble.get(), Use, MarkupKind::PlainText);
EXPECT_FALSE(Result.signatures.empty());
EXPECT_EQ(Result.signatures[0].label, "printA(int a) -> void");
EXPECT_EQ(Result.signatures[0].parameters[0].labelString, "int a");
}

} // namespace
} // namespace clang::clangd

Expand Down

0 comments on commit af47038

Please sign in to comment.