[clang][modules] Only avoid pruning module maps when asked to (llvm#89428)

Pruning non-affecting module maps is useful even when passing module
maps explicitly via `-fmodule-map-file=<path>`. For this situation, this
patch reinstates the behavior we had prior to llvm#87849. For the situation
where the explicit module map file arguments were generated by the
dependency scanner (which already pruned the non-affecting ones), this
patch introduces new `-cc1` flag
`-fno-modules-prune-non-affecting-module-map-files` that avoids the
extra work.

(cherry picked from commit 3ea5dff)

Author: jansvoboda11

clang/include/clang/Driver/Options.td

@@ -2579,6 +2579,11 @@ defm modules_skip_header_search_paths : BoolFOption<"modules-skip-header-search-
     HeaderSearchOpts<"ModulesSkipHeaderSearchPaths">, DefaultFalse,
     PosFlag<SetTrue, [], "Disable writing header search paths">,
     NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
+def fno_modules_prune_non_affecting_module_map_files :
+    Flag<["-"], "fno-modules-prune-non-affecting-module-map-files">,
+    Group<f_Group>, Flags<[CC1Option]>,
+    MarshallingInfoNegativeFlag<HeaderSearchOpts<"ModulesPruneNonAffectingModuleMaps">>,
+    HelpText<"Do not prune non-affecting module map files when writing module files">;
 
 def fincremental_extensions :
   Flag<["-"], "fincremental-extensions">,

clang/include/clang/Lex/HeaderSearchOptions.h

@@ -228,6 +228,9 @@ class HeaderSearchOptions {
   /// Primarily used to speed up deserialization during dependency scanning.
   unsigned ModulesSkipPragmaDiagnosticMappings : 1;
 
+  /// Whether to prune non-affecting module map files from PCM files.
+  unsigned ModulesPruneNonAffectingModuleMaps : 1;
+
   unsigned ModulesHashContent : 1;
 
   /// Whether we should include all things that could impact the module in the
@@ -252,7 +255,8 @@ class HeaderSearchOptions {
         ModulesValidateDiagnosticOptions(true),
         ModulesSkipDiagnosticOptions(false),
         ModulesSkipHeaderSearchPaths(false),
-        ModulesSkipPragmaDiagnosticMappings(false), ModulesHashContent(false),
+        ModulesSkipPragmaDiagnosticMappings(false),
+        ModulesPruneNonAffectingModuleMaps(true), ModulesHashContent(false),
         ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false) {}
 
   /// AddPath - Add the \p Path path to the specified \p Group list.

clang/lib/Serialization/ASTWriter.cpp

@@ -163,9 +163,9 @@ namespace {
 
 std::optional<std::set<const FileEntry *>>
 GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
-  // Without implicit module map search, there's no good reason to know about
-  // any module maps that are not affecting.
-  if (!PP.getHeaderSearchInfo().getHeaderSearchOpts().ImplicitModuleMaps)
+  if (!PP.getHeaderSearchInfo()
+           .getHeaderSearchOpts()
+           .ModulesPruneNonAffectingModuleMaps)
     return std::nullopt;
 
   SmallVector<const Module *> ModulesToProcess{RootModule};

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

@@ -185,6 +185,11 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
   CI.resetNonModularOptions();
   CI.clearImplicitModuleBuildOptions();
 
+  // The scanner takes care to avoid passing non-affecting module maps to the
+  // explicit compiles. No need to do extra work just to find out there are no
+  // module map files to prune.
+  CI.getHeaderSearchOpts().ModulesPruneNonAffectingModuleMaps = false;
+
   // Remove options incompatible with explicit module build or are likely to
   // differ between identical modules discovered from different translation
   // units.

clang/test/ClangScanDeps/modules-full.cpp

@@ -33,6 +33,7 @@
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1"
 // CHECK:              "-emit-module"
+// CHECK:              "-fno-modules-prune-non-affecting-module-map-files"
 // CHECK:              "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK-NOT:          "-fimplicit-module-maps"
 // CHECK:              "-fmodule-name=header1"
@@ -51,6 +52,7 @@
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1",
 // CHECK:              "-emit-module",
+// CHECK:              "-fno-modules-prune-non-affecting-module-map-files"
 // CHECK-NOT:          "-fimplicit-module-maps",
 // CHECK:              "-fmodule-name=header1",
 // CHECK:              "-fno-implicit-modules",
@@ -68,6 +70,7 @@
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1",
 // CHECK:              "-emit-module",
+// CHECK:              "-fno-modules-prune-non-affecting-module-map-files"
 // CHECK:              "-fmodule-name=header2",
 // CHECK-NOT:          "-fimplicit-module-maps",
 // CHECK:              "-fno-implicit-modules",

clang/test/Modules/add-remove-irrelevant-module-map.m

@@ -0,0 +1,62 @@
+// Check that the presence of non-affecting module map files does not affect the
+// contents of PCM files.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- a/module.modulemap
+module a {}
+
+//--- b/module.modulemap
+module b {}
+
+//--- c/module.modulemap
+module c { header "c.h" }
+//--- c/c.h
+@import b;
+
+//--- tu.m
+@import c;
+
+//--- explicit-mms-common-args.rsp
+-fmodule-map-file=b/module.modulemap -fmodule-map-file=c/module.modulemap -fmodules -fmodules-cache-path=cache -fdisable-module-hash -fsyntax-only tu.m
+//--- implicit-search-args.rsp
+-I a -I b -I c -fimplicit-module-maps -fmodules -fmodules-cache-path=cache -fdisable-module-hash -fsyntax-only tu.m
+//--- implicit-search-args.rsp-end
+
+// Test with explicit module map files.
+//
+// RUN: %clang_cc1 -working-directory %t @%t/explicit-mms-common-args.rsp
+// RUN: mv %t/cache %t/cache-explicit-no-a-prune
+// RUN: %clang_cc1 -working-directory %t @%t/explicit-mms-common-args.rsp -fno-modules-prune-non-affecting-module-map-files
+// RUN: mv %t/cache %t/cache-explicit-no-a-keep
+//
+// RUN: %clang_cc1 -working-directory %t -fmodule-map-file=a/module.modulemap @%t/explicit-mms-common-args.rsp
+// RUN: mv %t/cache %t/cache-explicit-a-prune
+// RUN: %clang_cc1 -working-directory %t -fmodule-map-file=a/module.modulemap @%t/explicit-mms-common-args.rsp -fno-modules-prune-non-affecting-module-map-files
+// RUN: mv %t/cache %t/cache-explicit-a-keep
+//
+// RUN: diff %t/cache-explicit-no-a-prune/c.pcm %t/cache-explicit-a-prune/c.pcm
+// RUN: not diff %t/cache-explicit-no-a-keep/c.pcm %t/cache-explicit-a-keep/c.pcm
+
+// Test with implicit module map search.
+//
+// RUN: %clang_cc1 -working-directory %t @%t/implicit-search-args.rsp
+// RUN: mv %t/cache %t/cache-implicit-no-a-prune
+// RUN: %clang_cc1 -working-directory %t @%t/implicit-search-args.rsp -fno-modules-prune-non-affecting-module-map-files
+// RUN: mv %t/cache %t/cache-implicit-no-a-keep
+//
+// FIXME: Instead of removing "a/module.modulemap" from the file system, we
+//        could drop the "-I a" search path argument in combination with the
+//        "-fmodules-skip-header-search-paths" flag. Unfortunately, that flag
+//        does not prevent serialization of the search path usage bit vector,
+//        making the files differ anyways.
+// RUN: rm %t/a/module.modulemap
+//
+// RUN: %clang_cc1 -working-directory %t @%t/implicit-search-args.rsp
+// RUN: mv %t/cache %t/cache-implicit-a-prune
+// RUN: %clang_cc1 -working-directory %t @%t/implicit-search-args.rsp -fno-modules-prune-non-affecting-module-map-files
+// RUN: mv %t/cache %t/cache-implicit-a-keep
+//
+// RUN: diff %t/cache-implicit-no-a-prune/c.pcm %t/cache-implicit-a-prune/c.pcm
+// RUN: not diff %t/cache-implicit-no-a-keep/c.pcm %t/cache-implicit-a-keep/c.pcm