[clang] a6ef363 - [clang][deps] Disable implicit module maps
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 12 02:32:44 PST 2022
Author: Jan Svoboda
Date: 2022-03-12T11:07:21+01:00
New Revision: a6ef3635461c7e689972337b281d0052331f6150
URL: https://github.com/llvm/llvm-project/commit/a6ef3635461c7e689972337b281d0052331f6150
DIFF: https://github.com/llvm/llvm-project/commit/a6ef3635461c7e689972337b281d0052331f6150.diff
LOG: [clang][deps] Disable implicit module maps
Since D113473, we don't report any module map files via `-fmodule-map-file=` in explicit builds. The ultimate goal here is to make sure Clang doesn't open/read/parse/evaluate unnecessary module maps.
However, implicit module maps still end up reading all reachable module maps. This patch disables implicit module maps in explicit builds.
Unfortunately, we still need to report some module map files that aren't encoded in PCM files of dependencies: module maps that are necessary to correctly evaluate includes in modules marked as `[no_undeclared_includes]`.
Depends on D120464.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D120465
Added:
clang/test/ClangScanDeps/modules-no-undeclared-includes.c
Modified:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-pch.c
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index c2e9541db68e0..d0fa474eda4eb 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -85,6 +85,10 @@ struct ModuleDeps {
/// on, not including transitive dependencies.
llvm::StringSet<> FileDeps;
+ /// A collection of absolute paths to module map files that this module needs
+ /// to know about.
+ std::vector<std::string> ModuleMapFileDeps;
+
/// A collection of prebuilt modular dependencies this module directly depends
/// on, not including transitive dependencies.
std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
@@ -122,13 +126,12 @@ struct ModuleDeps {
};
namespace detail {
-/// Collect the paths of PCM and module map files for the modules in \c Modules
-/// transitively.
-void collectPCMAndModuleMapPaths(
+/// Collect the paths of PCM for the modules in \c Modules transitively.
+void collectPCMPaths(
llvm::ArrayRef<ModuleID> Modules,
std::function<StringRef(ModuleID)> LookupPCMPath,
std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
- std::vector<std::string> &PCMPaths, std::vector<std::string> &ModMapPaths);
+ std::vector<std::string> &PCMPaths);
} // namespace detail
class ModuleDepCollector;
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 2723d4742819a..12602fccc92e0 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -19,9 +19,8 @@ std::vector<std::string> FullDependencies::getCommandLine(
std::vector<std::string> Ret = getCommandLineWithoutModulePaths();
std::vector<std::string> PCMPaths;
- std::vector<std::string> ModMapPaths;
- dependencies::detail::collectPCMAndModuleMapPaths(
- ClangModuleDeps, LookupPCMPath, LookupModuleDeps, PCMPaths, ModMapPaths);
+ dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath,
+ LookupModuleDeps, PCMPaths);
for (const std::string &PCMPath : PCMPaths)
Ret.push_back("-fmodule-file=" + PCMPath);
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 086215e7a573d..0b77ba8f9b0e8 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -50,11 +50,14 @@ CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
CI.getLangOpts()->ImplicitModules = false;
+ CI.getHeaderSearchOpts().ImplicitModuleMaps = false;
// Report the prebuilt modules this module uses.
for (const auto &PrebuiltModule : Deps.PrebuiltModuleDeps)
CI.getFrontendOpts().ModuleFiles.push_back(PrebuiltModule.PCMFile);
+ CI.getFrontendOpts().ModuleMapFiles = Deps.ModuleMapFileDeps;
+
Optimize(CI);
// The original invocation probably didn't have strict context hash enabled.
@@ -94,9 +97,9 @@ std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind);
FrontendOpts.OutputFile = std::string(LookupPCMPath(ID));
- dependencies::detail::collectPCMAndModuleMapPaths(
- ClangModuleDeps, LookupPCMPath, LookupModuleDeps,
- FrontendOpts.ModuleFiles, FrontendOpts.ModuleMapFiles);
+ dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath,
+ LookupModuleDeps,
+ FrontendOpts.ModuleFiles);
return serializeCompilerInvocation(CI);
}
@@ -106,11 +109,11 @@ ModuleDeps::getCanonicalCommandLineWithoutModulePaths() const {
return serializeCompilerInvocation(BuildInvocation);
}
-void dependencies::detail::collectPCMAndModuleMapPaths(
+void dependencies::detail::collectPCMPaths(
llvm::ArrayRef<ModuleID> Modules,
std::function<StringRef(ModuleID)> LookupPCMPath,
std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
- std::vector<std::string> &PCMPaths, std::vector<std::string> &ModMapPaths) {
+ std::vector<std::string> &PCMPaths) {
llvm::StringSet<> AlreadyAdded;
std::function<void(llvm::ArrayRef<ModuleID>)> AddArgs =
@@ -122,8 +125,6 @@ void dependencies::detail::collectPCMAndModuleMapPaths(
// Depth first traversal.
AddArgs(M.ClangModuleDeps);
PCMPaths.push_back(LookupPCMPath(MID).str());
- if (!M.ClangModuleMapFile.empty())
- ModMapPaths.push_back(M.ClangModuleMapFile);
}
};
@@ -262,6 +263,42 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
MD.FileDeps.insert(IF.getFile()->getName());
});
+ // We usually don't need to list the module map files of our dependencies when
+ // building a module explicitly: their semantics will be deserialized from PCM
+ // files.
+ //
+ // However, some module maps loaded implicitly during the dependency scan can
+ // describe anti-dependencies. That happens when this module, let's call it
+ // M1, is marked as '[no_undeclared_includes]' and tries to access a header
+ // "M2/M2.h" from another module, M2, but doesn't have a 'use M2;'
+ // declaration. The explicit build needs the module map for M2 so that it
+ // knows that textually including "M2/M2.h" is not allowed.
+ // E.g., '__has_include("M2/M2.h")' should return false, but without M2's
+ // module map the explicit build would return true.
+ //
+ // An alternative approach would be to tell the explicit build what its
+ // textual dependencies are, instead of having it re-discover its
+ // anti-dependencies. For example, we could create and use an `-ivfs-overlay`
+ // with `fall-through: false` that explicitly listed the dependencies.
+ // However, that's more complicated to implement and harder to reason about.
+ if (M->NoUndeclaredIncludes) {
+ // We don't have a good way to determine which module map described the
+ // anti-dependency (let alone what's the corresponding top-level module
+ // map). We simply specify all the module maps in the order they were loaded
+ // during the implicit build during scan.
+ // TODO: Resolve this by serializing and only using Module::UndeclaredUses.
+ MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
+ *MF, [&](const FileEntry *FE) {
+ if (FE->getName().endswith("__inferred_module.map"))
+ return;
+ // The top-level modulemap of this module will be the input file. We
+ // don't need to specify it as a module map.
+ if (FE == ModuleMap)
+ return;
+ MD.ModuleMapFileDeps.push_back(FE->getName().str());
+ });
+ }
+
// Add direct prebuilt module dependencies now, so that we can use them when
// creating a CompilerInvocation and computing context hash for this
// ModuleDeps instance.
diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp
index 2a7b6625bf0f1..9802f365a1005 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -50,6 +50,7 @@
// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}"
// CHECK-ABS: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
// CHECK-CUSTOM: "-fmodule-file=[[PREFIX]]/custom/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
+// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-name=header1"
// CHECK: "-fno-implicit-modules"
// CHECK: ],
@@ -66,6 +67,7 @@
// CHECK-NEXT: "command-line": [
// CHECK-NEXT: "-cc1",
// CHECK: "-emit-module",
+// CHECK-NOT: "-fimplicit-module-maps",
// CHECK: "-fmodule-name=header1",
// CHECK: "-fno-implicit-modules",
// CHECK: ],
@@ -83,6 +85,7 @@
// CHECK-NEXT: "-cc1",
// CHECK: "-emit-module",
// CHECK: "-fmodule-name=header2",
+// CHECK-NOT: "-fimplicit-module-maps",
// CHECK: "-fno-implicit-modules",
// CHECK: ],
// CHECK-NEXT: "context-hash": "[[HASH_H2_DINCLUDE]]",
diff --git a/clang/test/ClangScanDeps/modules-inferred.m b/clang/test/ClangScanDeps/modules-inferred.m
index 3fdf4edf5aa1a..1725b147f6d08 100644
--- a/clang/test/ClangScanDeps/modules-inferred.m
+++ b/clang/test/ClangScanDeps/modules-inferred.m
@@ -25,6 +25,7 @@
// CHECK-NEXT: "command-line": [
// CHECK-NEXT: "-cc1",
// CHECK: "-emit-module",
+// CHECK-NOT: "-fimplicit-module-maps",
// CHECK: "-fmodule-name=Inferred",
// CHECK: "-fno-implicit-modules",
// CHECK: ],
diff --git a/clang/test/ClangScanDeps/modules-no-undeclared-includes.c b/clang/test/ClangScanDeps/modules-no-undeclared-includes.c
new file mode 100644
index 0000000000000..005c279c9acfe
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-no-undeclared-includes.c
@@ -0,0 +1,72 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- undeclared/module.modulemap
+module Undeclared { header "undeclared.h" }
+
+//--- undeclared/undeclared.h
+
+//--- module.modulemap
+module User [no_undeclared_includes] { header "user.h" }
+
+//--- user.h
+#if __has_include("undeclared.h")
+#error Unreachable. Undeclared comes from a module that's not 'use'd, meaning the compiler should pretend it doesn't exist.
+#endif
+
+//--- test.c
+#include "user.h"
+
+//--- cdb.json.template
+[{
+ "directory": "DIR",
+ "command": "clang -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -IDIR/undeclared -c DIR/test.c -o DIR/test.o",
+ "file": "DIR/test.c"
+}]
+
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN: -generate-modules-path-args -module-files-dir %t/build > %t/result.json
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%t
+
+// CHECK: {
+// CHECK-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/undeclared/module.modulemap"
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/undeclared/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/user.h"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "User"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-context-hash": "{{.*}}"
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}"
+// CHECK-NEXT: "module-name": "User"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/test.c"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
+// CHECK-NEXT: }
+// CHECK: ]
+// CHECK-NEXT: }
+
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result.json --module-name=User > %t/User.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result.json --tu-index=0 > %t/tu.rsp
+//
+// RUN: %clang @%t/User.cc1.rsp
+// RUN: %clang @%t/tu.rsp
diff --git a/clang/test/ClangScanDeps/modules-pch.c b/clang/test/ClangScanDeps/modules-pch.c
index 89b6b6f9b0980..9ac43c17d7832 100644
--- a/clang/test/ClangScanDeps/modules-pch.c
+++ b/clang/test/ClangScanDeps/modules-pch.c
@@ -26,6 +26,7 @@
// CHECK-PCH-NEXT: "-cc1"
// CHECK-PCH: "-emit-module"
// CHECK-PCH: "-fmodules"
+// CHECK-PCH-NOT: "-fimplicit-module-maps"
// CHECK-PCH: "-fmodule-name=ModCommon1"
// CHECK-PCH: "-fno-implicit-modules"
// CHECK-PCH: ],
@@ -43,6 +44,7 @@
// CHECK-PCH-NEXT: "-cc1"
// CHECK-PCH: "-emit-module"
// CHECK-PCH: "-fmodules"
+// CHECK-PCH-NOT: "-fimplicit-module-maps"
// CHECK-PCH: "-fmodule-name=ModCommon2"
// CHECK-PCH: "-fno-implicit-modules"
// CHECK-PCH: ],
@@ -66,6 +68,7 @@
// CHECK-PCH: "-emit-module"
// CHECK-PCH: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm"
// CHECK-PCH: "-fmodules"
+// CHECK-PCH-NOT: "-fimplicit-module-maps"
// CHECK-PCH: "-fmodule-name=ModPCH"
// CHECK-PCH: "-fno-implicit-modules"
// CHECK-PCH: ],
@@ -139,6 +142,7 @@
// CHECK-TU-NEXT: "command-line": [
// CHECK-TU-NEXT: "-cc1",
// CHECK-TU: "-emit-module",
+// CHECK-TU-NOT: "-fimplicit-module-maps",
// CHECK-TU: "-fmodule-name=ModTU",
// CHECK-TU: "-fno-implicit-modules",
// CHECK-TU: ],
@@ -202,6 +206,7 @@
// CHECK-TU-WITH-COMMON-NEXT: "-cc1",
// CHECK-TU-WITH-COMMON: "-emit-module",
// CHECK-TU-WITH-COMMON: "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon1-{{.*}}.pcm",
+// CHECK-TU-WITH-COMMON-NOT: "-fimplicit-module-maps",
// CHECK-TU-WITH-COMMON: "-fmodule-name=ModTUWithCommon",
// CHECK-TU-WITH-COMMON: "-fno-implicit-modules",
// CHECK-TU-WITH-COMMON: ],
More information about the cfe-commits
mailing list