This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa6ef3635461c: [clang][deps] Disable implicit module maps
(authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D120465?vs=413384&id=414814#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120465/new/
https://reviews.llvm.org/D120465
Files:
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-no-undeclared-includes.c
clang/test/ClangScanDeps/modules-pch.c
Index: clang/test/ClangScanDeps/modules-pch.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch.c
+++ 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: ],
Index: clang/test/ClangScanDeps/modules-no-undeclared-includes.c
===================================================================
--- /dev/null
+++ 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
Index: clang/test/ClangScanDeps/modules-inferred.m
===================================================================
--- clang/test/ClangScanDeps/modules-inferred.m
+++ 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: ],
Index: clang/test/ClangScanDeps/modules-full.cpp
===================================================================
--- clang/test/ClangScanDeps/modules-full.cpp
+++ 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]]",
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -50,11 +50,14 @@
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 @@
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 @@
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 @@
// 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 @@
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.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -19,9 +19,8 @@
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);
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -85,6 +85,10 @@
/// 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 @@
};
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
