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
  • [PATCH] D120465: [... Jan Svoboda via Phabricator via cfe-commits

Reply via email to