[clang-tools-extra] [clangd] [C++ Modules] Skip PCH when TU imports modules (PR #187432)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 19 22:08:34 PDT 2026
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/187432
>From e7f762595cda45e1ddda2022e8788cee2198c695 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 19 Mar 2026 11:16:44 +0800
Subject: [PATCH] [clangd] [C++ Modules] Skip PCH when TU imports modules
Close https://github.com/llvm/llvm-project/issues/181770
PCH doesn't preserve module visibility when modules are imported
through #include'd headers, causing incorrect module_unimported_use
diagnostics. Set preamble bounds to 0 for such TUs to avoid the issue.
And also, I concern there are many problems mixing using PCH and C++20
modules. Although the root cause live in the clang side, we can disable
them in clangd right now to give users better diagnostics.
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 10 ++
clang-tools-extra/clangd/ModulesBuilder.h | 2 +
clang-tools-extra/clangd/ParsedAST.cpp | 17 +++-
.../unittests/PrerequisiteModulesTest.cpp | 99 +++++++++++++++++++
4 files changed, 124 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 950392b91ac7c..c2a95cff5f9fe 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -650,6 +650,16 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
return llvm::Error::success();
}
+bool ModulesBuilder::hasRequiredModules(PathRef File) {
+ std::unique_ptr<ProjectModules> MDB = Impl->getCDB().getProjectModules(File);
+ if (!MDB)
+ return false;
+
+ CachingProjectModules CachedMDB(std::move(MDB),
+ Impl->getProjectModulesCache());
+ return !CachedMDB.getRequiredModules(File).empty();
+}
+
std::unique_ptr<PrerequisiteModules>
ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
const ThreadsafeFS &TFS) {
diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h
index f40a9006e9169..b4d1278e079df 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.h
+++ b/clang-tools-extra/clangd/ModulesBuilder.h
@@ -94,6 +94,8 @@ class ModulesBuilder {
ModulesBuilder &operator=(const ModulesBuilder &) = delete;
ModulesBuilder &operator=(ModulesBuilder &&) = delete;
+ bool hasRequiredModules(PathRef File);
+
std::unique_ptr<PrerequisiteModules>
buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS);
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 4e873f1257a17..bf4fb5c909bf3 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -430,6 +430,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
const PrecompiledPreamble *PreamblePCH =
Preamble ? &Preamble->Preamble : nullptr;
+ // Skip PCH when the TU requires C++20 named modules. Mixing PCH and modules
+ // may cause issues (e.g., module_decl_not_at_start error when the preamble
+ // contains 'module;' with #include directives followed by 'import').
+ // Here is a simple workaround for better user experience.
+ if (PreamblePCH && Inputs.ModulesManager &&
+ Inputs.ModulesManager->hasRequiredModules(Filename)) {
+ PreamblePCH = nullptr;
+ }
+
// This is on-by-default in windows to allow parsing SDK headers, but it
// breaks many features. Disable it for the main-file (not preamble).
CI->getLangOpts().DelayedTemplateParsing = false;
@@ -459,7 +468,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// dropped later on to not pay for extra latency by processing them.
DiagnosticConsumer *DiagConsumer = &ASTDiags;
IgnoreDiagnostics DropDiags;
- if (Preamble) {
+ if (PreamblePCH) {
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
}
@@ -663,7 +672,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
IncludeStructure Includes;
include_cleaner::PragmaIncludes PI;
// If we are using a preamble, copy existing includes.
- if (Preamble) {
+ if (Preamble && Patch) {
Includes = Preamble->Includes;
Includes.MainFileIncludes = Patch->preambleIncludes();
// Replay the preamble includes so that clang-tidy checks can see them.
@@ -682,7 +691,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// with non-preamble macros below.
MainFileMacros Macros;
std::vector<PragmaMark> Marks;
- if (Preamble) {
+ if (Patch) {
Macros = Patch->mainFileMacros();
Marks = Patch->marks();
}
@@ -744,7 +753,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// FIXME: Also skip generation of diagnostics altogether to speed up ast
// builds when we are patching a stale preamble.
// Add diagnostics from the preamble, if any.
- if (Preamble)
+ if (Patch)
llvm::append_range(Diags, Patch->patchedDiags());
// Finally, add diagnostics coming from the AST.
{
diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
index 3132959a5967c..4728831eb8bd2 100644
--- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
+++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
@@ -673,6 +673,105 @@ import M;
EXPECT_EQ(HS.PrebuiltModuleFiles, HS2.PrebuiltModuleFiles);
}
+// Test that module visibility is preserved when importing modules through
+// #include'd headers. See https://github.com/llvm/llvm-project/issues/181770
+TEST_F(PrerequisiteModulesTests, ModuleImportThroughInclude) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("M.cppm", R"cpp(
+export module M;
+export struct MyType {};
+ )cpp");
+
+ CDB.addFile("Header.h", R"cpp(
+import M;
+ )cpp");
+
+ CDB.addFile("Use.cpp", R"cpp(
+#include "Header.h"
+void use() {
+ MyType t;
+}
+ )cpp");
+
+ 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 AST = ParsedAST::build(getFullPath("Use.cpp"), Use, std::move(CI), {},
+ Preamble);
+ EXPECT_TRUE(AST);
+
+ // Verify that MyType is usable (no module_unimported_use error)
+ const NamedDecl &D = findDecl(*AST, "MyType");
+ EXPECT_TRUE(D.isFromASTFile());
+}
+
+// Test that mixing PCH and C++20 named modules doesn't cause
+// module_decl_not_at_start error when the preamble contains 'module;' with
+// #include directives followed by 'import'. See
+// https://github.com/llvm/llvm-project/issues/181770
+TEST_F(PrerequisiteModulesTests, PCHWithNamedModulesIncludeAndImport) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+ )cpp");
+
+ CDB.addFile("myheader.h", R"cpp(
+int x;
+ )cpp");
+
+ // Module B has 'module;' with #include, then imports A.
+ // This used to trigger module_decl_not_at_start error.
+ CDB.addFile("B.cppm", R"cpp(
+module;
+
+#include "myheader.h"
+
+export module B;
+
+import A;
+ )cpp");
+
+ ModulesBuilder Builder(CDB);
+
+ ParseInputs BInput = getInputs("B.cppm", CDB);
+ BInput.ModulesManager = &Builder;
+
+ std::unique_ptr<CompilerInvocation> CI =
+ buildCompilerInvocation(BInput, DiagConsumer);
+ EXPECT_TRUE(CI);
+
+ auto Preamble =
+ buildPreamble(getFullPath("B.cppm"), *CI, BInput, /*InMemory=*/true,
+ /*Callback=*/nullptr);
+ EXPECT_TRUE(Preamble);
+ EXPECT_TRUE(Preamble->RequiredModules);
+
+ auto AST = ParsedAST::build(getFullPath("B.cppm"), BInput, std::move(CI), {},
+ Preamble);
+ EXPECT_TRUE(AST);
+
+ // Verify no module_decl_not_at_start error
+ for (const auto &D : AST->getDiagnostics()) {
+ EXPECT_FALSE(llvm::StringRef(D.Message).contains(
+ "module declaration must occur at the start"))
+ << "Unexpected error: " << D.Message;
+ }
+}
+
} // namespace
} // namespace clang::clangd
More information about the cfe-commits
mailing list