jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, rsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
During 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 asking the `ModuleManager` for the module
instead of relying on the `ASTReader` listener. If the modules were loaded as
part of loading the PCH, `ModuleManager` will know about them even though
`ASTReader` didn't read them for the second time.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111560
Files:
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Serialization/ModuleFile.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/modules-pch.c
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
Index: clang/test/Modules/pch-shared-module.c
===================================================================
--- /dev/null
+++ 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
Index: clang/test/Modules/Inputs/pch-shared-module/pch.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pch-shared-module/pch.h
@@ -0,0 +1 @@
+#include "mod.h"
Index: clang/test/Modules/Inputs/pch-shared-module/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/pch-shared-module/module.modulemap
@@ -0,0 +1 @@
+module mod { header "mod.h" }
Index: clang/test/ClangScanDeps/modules-pch.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch.c
+++ 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"
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -38,7 +38,7 @@
};
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);
}
Index: clang/lib/Serialization/ModuleFile.cpp
===================================================================
--- clang/lib/Serialization/ModuleFile.cpp
+++ clang/lib/Serialization/ModuleFile.cpp
@@ -28,6 +28,15 @@
delete static_cast<ASTSelectorLookupTable *>(SelectorLookupTable);
}
+void ModuleFile::collectTransitiveImports(std::vector<ModuleFile *> &Files) {
+ SmallVector<serialization::ModuleFile *, 4> Stack{this};
+ while (!Stack.empty()) {
+ serialization::ModuleFile *MF = Stack.pop_back_val();
+ Stack.insert(Stack.end(), MF->Imports.begin(), MF->Imports.end());
+ Files.push_back(MF);
+ }
+}
+
template<typename Key, typename Offset, unsigned InitialCapacity>
static void
dumpLocalRemap(StringRef Name,
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1626,13 +1626,6 @@
CI.getPreprocessor().getIdentifierInfo(ModuleName));
}
- void registerAll() {
- ModuleMap &MM = CI.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- for (auto *II : LoadedModules)
- MM.cacheModuleLoad(*II, MM.findModule(II->getName()));
- LoadedModules.clear();
- }
-
void markAllUnavailable() {
for (auto *II : LoadedModules) {
if (Module *M = CI.getPreprocessor()
@@ -1658,6 +1651,16 @@
}
};
+ auto RegisterAll = [&](serialization::ModuleFile *TopMF) {
+ std::vector<serialization::ModuleFile *> AllMFs{TopMF};
+ TopMF->collectTransitiveImports(AllMFs);
+ for (serialization::ModuleFile *MF : AllMFs) {
+ ModuleMap &MM = getPreprocessor().getHeaderSearchInfo().getModuleMap();
+ MM.cacheModuleLoad(*getPreprocessor().getIdentifierInfo(MF->ModuleName),
+ MM.findModule(MF->ModuleName));
+ }
+ };
+
// If we don't already have an ASTReader, create one now.
if (!TheASTReader)
createASTReader();
@@ -1681,7 +1684,7 @@
case ASTReader::Success:
// We successfully loaded the module file; remember the set of provided
// modules so that we don't try to load implicit modules for them.
- ListenerRef.registerAll();
+ RegisterAll(TheASTReader->getModuleManager().lookupByFileName(FileName));
return true;
case ASTReader::ConfigurationMismatch:
Index: clang/include/clang/Serialization/ModuleFile.h
===================================================================
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -503,6 +503,9 @@
Kind == MK_PrebuiltModule;
}
+ /// Collect all transitive imports of this module file.
+ void collectTransitiveImports(std::vector<ModuleFile *> &Files);
+
/// Dump debugging output for this module.
void dump();
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
