[clang] [clang][modules] Allow module map files with textual headers be non-affecting (PR #89441)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 19 12:15:42 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-modules
Author: Jan Svoboda (jansvoboda11)
<details>
<summary>Changes</summary>
Depends on #<!-- -->89428.
---
Full diff: https://github.com/llvm/llvm-project/pull/89441.diff
8 Files Affected:
- (modified) clang/include/clang/Driver/Options.td (+5)
- (modified) clang/include/clang/Lex/HeaderSearchOptions.h (+6-1)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+9-7)
- (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+5)
- (modified) clang/test/ClangScanDeps/modules-full.cpp (+3)
- (removed) clang/test/Modules/add-remove-irrelevant-module-map.m (-32)
- (added) clang/test/Modules/prune-non-affecting-module-map-files-textual.c (+26)
- (added) clang/test/Modules/prune-non-affecting-module-map-files.m (+62)
``````````diff
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d83031b05cebc4..a3d2863bc7507e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3100,6 +3100,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<[]>, Visibility<[CC1Option]>,
+ MarshallingInfoNegativeFlag<HeaderSearchOpts<"ModulesPruneNonAffectingModuleMaps">>,
+ HelpText<"Do not prune non-affecting module map files when writing module files">;
def fincremental_extensions :
Flag<["-"], "fincremental-extensions">,
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 637dc77e5d957e..e4437ac0e35263 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -252,6 +252,10 @@ class HeaderSearchOptions {
LLVM_PREFERRED_TYPE(bool)
unsigned ModulesSkipPragmaDiagnosticMappings : 1;
+ /// Whether to prune non-affecting module map files from PCM files.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned ModulesPruneNonAffectingModuleMaps : 1;
+
LLVM_PREFERRED_TYPE(bool)
unsigned ModulesHashContent : 1;
@@ -280,7 +284,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.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 6dd87b5d200db6..4825c245a4b846 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -166,9 +166,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};
@@ -187,7 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
continue;
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+ if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+ !HFI->isCompilingModuleHeader))
continue;
for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -2055,11 +2056,12 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
// Get the file info. Skip emitting this file if we have no information on
// it as a header file (in which case HFI will be null) or if it hasn't
- // changed since it was loaded. Also skip it if it's for a modular header
- // from a different module; in that case, we rely on the module(s)
+ // changed since it was loaded. Also skip it if it's for a non-excluded
+ // header from a different module; in that case, we rely on the module(s)
// containing the header to provide this information.
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+ if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+ !HFI->isCompilingModuleHeader))
continue;
// Massage the file path into an appropriate form.
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index e19f19b2528c15..f46324ee9989eb 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -179,6 +179,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.
diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp
index 59efef0ecbaa64..a00a431eb56911 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/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",
diff --git a/clang/test/Modules/add-remove-irrelevant-module-map.m b/clang/test/Modules/add-remove-irrelevant-module-map.m
deleted file mode 100644
index 7e3e58037e6f21..00000000000000
--- a/clang/test/Modules/add-remove-irrelevant-module-map.m
+++ /dev/null
@@ -1,32 +0,0 @@
-// 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 {}
-
-//--- module.modulemap
-module m { header "m.h" }
-//--- m.h
- at import c;
-
-//--- test-simple.m
-// expected-no-diagnostics
- at import m;
-
-// Build modules with the non-affecting "a/module.modulemap".
-// RUN: %clang_cc1 -I %t/a -I %t/b -I %t/c -I %t -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -fdisable-module-hash %t/test-simple.m -verify
-// RUN: mv %t/cache %t/cache-with
-
-// Build modules without the non-affecting "a/module.modulemap".
-// RUN: rm -rf %t/a/module.modulemap
-// RUN: %clang_cc1 -I %t/a -I %t/b -I %t/c -I %t -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -fdisable-module-hash %t/test-simple.m -verify
-// RUN: mv %t/cache %t/cache-without
-
-// Check that the PCM files are bit-for-bit identical.
-// RUN: diff %t/cache-with/m.pcm %t/cache-without/m.pcm
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
new file mode 100644
index 00000000000000..8f8f00560b1834
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -0,0 +1,26 @@
+// This test checks that a module map with a textual header can be marked as
+// non-affecting.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- A.modulemap
+module A { textual header "A.h" }
+//--- B.modulemap
+module B { header "B.h" export * }
+//--- A.h
+typedef int A_int;
+//--- B.h
+#include "A.h"
+typedef A_int B_int;
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A.pcm \
+// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
+// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
+// RUN: -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: diff %t/B0.pcm %t/B1.pcm
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files.m b/clang/test/Modules/prune-non-affecting-module-map-files.m
new file mode 100644
index 00000000000000..ba2b3a306eaf46
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files.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
+ at import b;
+
+//--- tu.m
+ at 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
``````````
</details>
https://github.com/llvm/llvm-project/pull/89441
More information about the cfe-commits
mailing list