Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Expand Down
7 changes: 6 additions & 1 deletion clang/include/clang/Lex/HeaderSearchOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions clang/test/ClangScanDeps/modules-full.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand All @@ -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",
Expand Down
32 changes: 0 additions & 32 deletions clang/test/Modules/add-remove-irrelevant-module-map.m

This file was deleted.

62 changes: 62 additions & 0 deletions clang/test/Modules/prune-non-affecting-module-map-files.m
Original file line number Diff line number Diff line change
@@ -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