jansvoboda11 created this revision. jansvoboda11 added reviewers: Bigcheese, dexonsmith. jansvoboda11 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Since D113473 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D120464>. Repository: rG LLVM Github Monorepo 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. @@ -96,7 +99,7 @@ dependencies::detail::collectPCMAndModuleMapPaths( ClangModuleDeps, LookupPCMPath, LookupModuleDeps, - FrontendOpts.ModuleFiles, FrontendOpts.ModuleMapFiles); + FrontendOpts.ModuleFiles); return serializeCompilerInvocation(CI); } @@ -110,7 +113,7 @@ 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,31 @@ 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 the current module is marked + // as '[no_undeclared_includes]', doesn't 'use' module from such module map, + // but tries to import it anyway. We need to tell the explicit build about + // such module map for it to have the same semantics as the implicit build. + 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. + 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); + 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. +/// Collect the paths of PCM for the modules in \c Modules transitively. void collectPCMAndModuleMapPaths( 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