[clang] 08c8016 - [clang][modules] Cache loads of modules imported by PCH
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 13 09:09:59 PDT 2021
Author: Jan Svoboda
Date: 2021-10-13T18:09:52+02:00
New Revision: 08c8016cfb2af9463514709271ae8c4ad6b19377
URL: https://github.com/llvm/llvm-project/commit/08c8016cfb2af9463514709271ae8c4ad6b19377
DIFF: https://github.com/llvm/llvm-project/commit/08c8016cfb2af9463514709271ae8c4ad6b19377.diff
LOG: [clang][modules] Cache loads of modules imported by PCH
During explicit modular build, PCM files are typically specified via the `-fmodule-file=<path>` command-line option. Early during the compilation, Clang uses the `ASTReader` to read their contents and caches the result so that the module isn't loaded implicitly later on. A listener is attached to the `ASTReader` to collect names of the modules read from the PCM files. However, if the PCM has already been loaded previously via PCH:
1. the `ASTReader` doesn't do anything for the second time,
2. the listener is not invoked at all,
3. the module load result is not cached,
4. the compilation fails when attempting to load the module implicitly later on.
This patch solves this problem by attaching the listener to the `ASTReader` for PCH reading as well.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D111560
Added:
clang/test/Modules/Inputs/pch-shared-module/mod.h
clang/test/Modules/Inputs/pch-shared-module/module.modulemap
clang/test/Modules/Inputs/pch-shared-module/pch.h
clang/test/Modules/pch-shared-module.c
Modified:
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/modules-pch.c
Removed:
################################################################################
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 634b120284b66..0e43fa404da70 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -564,18 +564,17 @@ namespace {
// We mark these as known and redirect any attempt to load that module to
// the files we were handed.
struct ReadModuleNames : ASTReaderListener {
- CompilerInstance &CI;
+ Preprocessor &PP;
llvm::SmallVector<IdentifierInfo*, 8> LoadedModules;
- ReadModuleNames(CompilerInstance &CI) : CI(CI) {}
+ ReadModuleNames(Preprocessor &PP) : PP(PP) {}
void ReadModuleName(StringRef ModuleName) override {
- LoadedModules.push_back(
- CI.getPreprocessor().getIdentifierInfo(ModuleName));
+ LoadedModules.push_back(PP.getIdentifierInfo(ModuleName));
}
void registerAll() {
- ModuleMap &MM = CI.getPreprocessor().getHeaderSearchInfo().getModuleMap();
+ ModuleMap &MM = PP.getHeaderSearchInfo().getModuleMap();
for (auto *II : LoadedModules)
MM.cacheModuleLoad(*II, MM.findModule(II->getName()));
LoadedModules.clear();
@@ -583,10 +582,8 @@ struct ReadModuleNames : ASTReaderListener {
void markAllUnavailable() {
for (auto *II : LoadedModules) {
- if (Module *M = CI.getPreprocessor()
- .getHeaderSearchInfo()
- .getModuleMap()
- .findModule(II->getName())) {
+ if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
+ II->getName())) {
M->HasIncompatibleModuleFile = true;
// Mark module as available if the only reason it was unavailable
@@ -651,6 +648,11 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
for (auto &Listener : DependencyCollectors)
Listener->attachToASTReader(*Reader);
+ auto Listener = std::make_unique<ReadModuleNames>(PP);
+ auto &ListenerRef = *Listener;
+ ASTReader::ListenerScope ReadModuleNamesListener(*Reader,
+ std::move(Listener));
+
switch (Reader->ReadAST(Path,
Preamble ? serialization::MK_Preamble
: serialization::MK_PCH,
@@ -660,6 +662,7 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
// Set the predefines buffer as suggested by the PCH reader. Typically, the
// predefines buffer will be empty.
PP.setPredefines(Reader->getSuggestedPredefines());
+ ListenerRef.registerAll();
return Reader;
case ASTReader::Failure:
@@ -675,6 +678,7 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
break;
}
+ ListenerRef.markAllUnavailable();
Context.setExternalSource(nullptr);
return nullptr;
}
@@ -1693,7 +1697,7 @@ bool CompilerInstance::loadModuleFile(StringRef FileName) {
SourceLocation())
<= DiagnosticsEngine::Warning;
- auto Listener = std::make_unique<ReadModuleNames>(*this);
+ auto Listener = std::make_unique<ReadModuleNames>(*PP);
auto &ListenerRef = *Listener;
ASTReader::ListenerScope ReadModuleNamesListener(*TheASTReader,
std::move(Listener));
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 391e68bb089f9..e426284a84366 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -38,7 +38,7 @@ FullDependencies::getAdditionalArgsWithoutModulePaths() const {
};
for (const PrebuiltModuleDep &PMD : PrebuiltModuleDeps) {
- Args.push_back("-fmodule-file=" + PMD.ModuleName + "=" + PMD.PCMFile);
+ Args.push_back("-fmodule-file=" + PMD.PCMFile);
Args.push_back("-fmodule-map-file=" + PMD.ModuleMapFile);
}
diff --git a/clang/test/ClangScanDeps/modules-pch.c b/clang/test/ClangScanDeps/modules-pch.c
index 1f368f9e87fbe..589f5d537b905 100644
--- a/clang/test/ClangScanDeps/modules-pch.c
+++ b/clang/test/ClangScanDeps/modules-pch.c
@@ -229,8 +229,7 @@
// CHECK-TU-WITH-COMMON-NEXT: "command-line": [
// CHECK-TU-WITH-COMMON-NEXT: "-fno-implicit-modules",
// CHECK-TU-WITH-COMMON-NEXT: "-fno-implicit-module-maps",
-// FIXME: Figure out why we need `=ModCommon2` here for Clang to pick up the PCM.
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=ModCommon2=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
+// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModTUWithCommon-{{.*}}.pcm",
// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
diff --git a/clang/test/Modules/Inputs/pch-shared-module/mod.h b/clang/test/Modules/Inputs/pch-shared-module/mod.h
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Modules/Inputs/pch-shared-module/module.modulemap b/clang/test/Modules/Inputs/pch-shared-module/module.modulemap
new file mode 100644
index 0000000000000..03cbffaeb1fb5
--- /dev/null
+++ b/clang/test/Modules/Inputs/pch-shared-module/module.modulemap
@@ -0,0 +1 @@
+module mod { header "mod.h" }
diff --git a/clang/test/Modules/Inputs/pch-shared-module/pch.h b/clang/test/Modules/Inputs/pch-shared-module/pch.h
new file mode 100644
index 0000000000000..01f145835c765
--- /dev/null
+++ b/clang/test/Modules/Inputs/pch-shared-module/pch.h
@@ -0,0 +1 @@
+#include "mod.h"
diff --git a/clang/test/Modules/pch-shared-module.c b/clang/test/Modules/pch-shared-module.c
new file mode 100644
index 0000000000000..fd090d6ddf142
--- /dev/null
+++ b/clang/test/Modules/pch-shared-module.c
@@ -0,0 +1,14 @@
+// rm -rf %t && mkdir %t
+
+// RUN: %clang_cc1 -fmodules -emit-module -fmodule-name=mod %S/Inputs/pch-shared-module/module.modulemap -o %t/mod.pcm
+
+// RUN: %clang_cc1 -fmodules -emit-pch %S/Inputs/pch-shared-module/pch.h -o %t/pch.h.gch \
+// RUN: -fmodule-file=%t/mod.pcm -fmodule-map-file=%S/Inputs/pch-shared-module/module.modulemap
+
+// Check that `mod.pcm` is loaded correctly, even though it's imported by the PCH as well.
+// RUN: %clang_cc1 -fmodules -fsyntax-only %s -include-pch %t/pch.h.gch -I %S/Inputs/pch-shared-module \
+// RUN: -fmodule-file=%t/mod.pcm -fmodule-map-file=%S/Inputs/pch-shared-module/module.modulemap -verify
+
+#include "mod.h"
+
+// expected-no-diagnostics
More information about the cfe-commits
mailing list