[clang][deps] Disable implicit module maps
authorJan Svoboda <jan_svoboda@apple.com>
Sat, 12 Mar 2022 10:04:17 +0000 (11:04 +0100)
committerJan Svoboda <jan_svoboda@apple.com>
Sat, 12 Mar 2022 10:07:21 +0000 (11:07 +0100)
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

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 [new file with mode: 0644]
clang/test/ClangScanDeps/modules-pch.c

index c2e9541..d0fa474 100644 (file)
@@ -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;
index 2723d47..12602fc 100644 (file)
@@ -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);
 
index 086215e..0b77ba8 100644 (file)
@@ -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.
index 2a7b662..9802f36 100644 (file)
@@ -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 3fdf4ed..1725b14 100644 (file)
@@ -25,6 +25,7 @@ inferred a = 0;
 // 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 (file)
index 0000000..005c279
--- /dev/null
@@ -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 89b6b6f..9ac43c1 100644 (file)
@@ -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:            ],
 // 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:            ],
 // 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:            ],