diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/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 ModuleMapFileDeps; + /// A collection of prebuilt modular dependencies this module directly depends /// on, not including transitive dependencies. std::vector 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 Modules, std::function LookupPCMPath, std::function LookupModuleDeps, - std::vector &PCMPaths, std::vector &ModMapPaths); + std::vector &PCMPaths); } // namespace detail class ModuleDepCollector; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -19,9 +19,8 @@ std::vector Ret = getCommandLineWithoutModulePaths(); std::vector PCMPaths; - std::vector 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 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/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 Modules, std::function LookupPCMPath, std::function LookupModuleDeps, - std::vector &PCMPaths, std::vector &ModMapPaths) { + std::vector &PCMPaths) { llvm::StringSet<> AlreadyAdded; std::function)> 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. diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp --- 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 --- 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 --- /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 --- 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: ],