Skip to content

Commit 49c5138

Browse files
jansvoboda11jansvoboda11
authored
[clang][modules] Allow not forcing validation of user headers (#139091)
Force-validation of user headers was implemented in acb803e to deal with files changing during build. The dependency scanner guarantees an immutable file system during single build session, so the validation is unnecessary. (We don't hit the disk too often due to the caching VFS, but even avoiding going to the cache and deserializing the input files makes sense.)
1 parent 51ca3cb commit 49c5138

File tree

8 files changed

+64
-7
lines changed

8 files changed

+64
-7
lines changed

‎clang/include/clang/Driver/Options.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3269,6 +3269,13 @@ def fmodules_disable_diagnostic_validation : Flag<["-"], "fmodules-disable-diagn
32693269
Group<i_Group>, Visibility<[ClangOption, CC1Option]>,
32703270
HelpText<"Disable validation of the diagnostic options when loading the module">,
32713271
MarshallingInfoNegativeFlag<HeaderSearchOpts<"ModulesValidateDiagnosticOptions">>;
3272+
defm modules_force_validate_user_headers : BoolOption<"f", "modules-force-validate-user-headers",
3273+
HeaderSearchOpts<"ModulesForceValidateUserHeaders">, DefaultTrue,
3274+
PosFlag<SetTrue, [], [], "Force">,
3275+
NegFlag<SetFalse, [], [CC1Option], "Do not force">,
3276+
BothFlags<[], [ClangOption],
3277+
" validation of user headers when repeatedly loading a module file within single build session">>,
3278+
Group<i_Group>;
32723279
defm modules_validate_system_headers : BoolOption<"f", "modules-validate-system-headers",
32733280
HeaderSearchOpts<"ModulesValidateSystemHeaders">, DefaultFalse,
32743281
PosFlag<SetTrue, [], [ClangOption, CC1Option],

‎clang/include/clang/Lex/HeaderSearchOptions.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,11 @@ class HeaderSearchOptions {
217217
LLVM_PREFERRED_TYPE(bool)
218218
unsigned ModulesValidateSystemHeaders : 1;
219219

220+
/// Whether to force the validation of user input files when a module is
221+
/// loaded (even despite the build session saying that is not necessary).
222+
LLVM_PREFERRED_TYPE(bool)
223+
unsigned ModulesForceValidateUserHeaders : 1;
224+
220225
// Whether the content of input files should be hashed and used to
221226
// validate consistency.
222227
LLVM_PREFERRED_TYPE(bool)
@@ -286,6 +291,7 @@ class HeaderSearchOptions {
286291
UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
287292
ModulesValidateOncePerBuildSession(false),
288293
ModulesValidateSystemHeaders(false),
294+
ModulesForceValidateUserHeaders(true),
289295
ValidateASTInputFilesContent(false),
290296
ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false),
291297
ModulesValidateDiagnosticOptions(true),

‎clang/include/clang/Serialization/ASTReader.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1091,9 +1091,12 @@ class ASTReader
10911091
/// from the current compiler instance.
10921092
bool AllowConfigurationMismatch;
10931093

1094-
/// Whether validate system input files.
1094+
/// Whether to validate system input files.
10951095
bool ValidateSystemInputs;
10961096

1097+
/// Whether to force the validation of user input files.
1098+
bool ForceValidateUserInputs;
1099+
10971100
/// Whether validate headers and module maps using hash based on contents.
10981101
bool ValidateASTInputFilesContent;
10991102

@@ -1767,6 +1770,7 @@ class ASTReader
17671770
bool AllowASTWithCompilerErrors = false,
17681771
bool AllowConfigurationMismatch = false,
17691772
bool ValidateSystemInputs = false,
1773+
bool ForceValidateUserInputs = true,
17701774
bool ValidateASTInputFilesContent = false,
17711775
bool UseGlobalIndex = true,
17721776
std::unique_ptr<llvm::Timer> ReadTimer = {});

‎clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -638,8 +638,9 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource(
638638
PP, ModCache, &Context, PCHContainerRdr, Extensions,
639639
Sysroot.empty() ? "" : Sysroot.data(), DisableValidation,
640640
AllowPCHWithCompilerErrors, /*AllowConfigurationMismatch*/ false,
641-
HSOpts.ModulesValidateSystemHeaders, HSOpts.ValidateASTInputFilesContent,
642-
UseGlobalModuleIndex));
641+
HSOpts.ModulesValidateSystemHeaders,
642+
HSOpts.ModulesForceValidateUserHeaders,
643+
HSOpts.ValidateASTInputFilesContent, UseGlobalModuleIndex));
643644

644645
// We need the external source to be set up before we read the AST, because
645646
// eagerly-deserialized declarations may use it.
@@ -1752,6 +1753,7 @@ void CompilerInstance::createASTReader() {
17521753
PPOpts.DisablePCHOrModuleValidation,
17531754
/*AllowASTWithCompilerErrors=*/FEOpts.AllowPCMWithCompilerErrors,
17541755
/*AllowConfigurationMismatch=*/false, HSOpts.ModulesValidateSystemHeaders,
1756+
HSOpts.ModulesForceValidateUserHeaders,
17551757
HSOpts.ValidateASTInputFilesContent,
17561758
getFrontendOpts().UseGlobalModuleIndex, std::move(ReadTimer));
17571759
if (hasASTConsumer()) {

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ void VerifyPCHAction::ExecuteAction() {
364364
DisableValidationForModuleKind::None,
365365
/*AllowASTWithCompilerErrors*/ false,
366366
/*AllowConfigurationMismatch*/ true,
367-
/*ValidateSystemInputs*/ true));
367+
/*ValidateSystemInputs*/ true, /*ForceValidateUserInputs*/ true));
368368

369369
Reader->ReadAST(getCurrentFile(),
370370
Preamble ? serialization::MK_Preamble

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3105,7 +3105,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
31053105
if (HSOpts.ModulesValidateOncePerBuildSession &&
31063106
F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp &&
31073107
F.Kind == MK_ImplicitModule)
3108-
N = NumUserInputs;
3108+
N = ForceValidateUserInputs ? NumUserInputs : 0;
31093109

31103110
for (unsigned I = 0; I < N; ++I) {
31113111
InputFile IF = getInputFile(F, I+1, Complain);
@@ -10974,6 +10974,7 @@ ASTReader::ASTReader(Preprocessor &PP, ModuleCache &ModCache,
1097410974
DisableValidationForModuleKind DisableValidationKind,
1097510975
bool AllowASTWithCompilerErrors,
1097610976
bool AllowConfigurationMismatch, bool ValidateSystemInputs,
10977+
bool ForceValidateUserInputs,
1097710978
bool ValidateASTInputFilesContent, bool UseGlobalIndex,
1097810979
std::unique_ptr<llvm::Timer> ReadTimer)
1097910980
: Listener(bool(DisableValidationKind & DisableValidationForModuleKind::PCH)
@@ -10989,6 +10990,7 @@ ASTReader::ASTReader(Preprocessor &PP, ModuleCache &ModCache,
1098910990
AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
1099010991
AllowConfigurationMismatch(AllowConfigurationMismatch),
1099110992
ValidateSystemInputs(ValidateSystemInputs),
10993+
ForceValidateUserInputs(ForceValidateUserInputs),
1099210994
ValidateASTInputFilesContent(ValidateASTInputFilesContent),
1099310995
UseGlobalIndex(UseGlobalIndex), CurrSwitchCaseStmts(&SwitchCaseStmts) {
1099410996
SourceMgr.setExternalSLocEntrySource(this);

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ class DependencyScanningAction : public tooling::ToolAction {
530530
ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
531531
ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings =
532532
true;
533+
ScanInstance.getHeaderSearchOpts().ModulesForceValidateUserHeaders = false;
533534

534535
// Avoid some checks and module map parsing when loading PCM files.
535536
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;

‎clang/test/Modules/fmodules-validate-once-per-build-session.c

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,44 @@
1919
// Compile the module.
2020
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
2121
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
22+
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
2223
// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
2324
// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
2425
// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
2526
// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
27+
// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
28+
// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
2629
// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-before.pcm
2730
// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-before.pcm
2831
// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-before-user.pcm
2932
// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-before-user.pcm
33+
// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-before-user-no-force.pcm
34+
// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-before-user-no-force.pcm
3035

3136
// ===
3237
// Use it, and make sure that we did not recompile it.
3338
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
3439
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
40+
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-use-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
3541
// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
3642
// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
3743
// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
3844
// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
45+
// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
46+
// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
3947
// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
4048
// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
4149
// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
4250
// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
51+
// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
52+
// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
4353

4454
// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
4555
// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
4656
// RUN: diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
4757
// RUN: diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
58+
// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
59+
// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
4860

4961
// ===
5062
// Change the sources.
@@ -53,31 +65,54 @@
5365

5466
// ===
5567
// Use the module, and make sure that we did not recompile it if foo.h or
56-
// module.modulemap are system files, even though the sources changed.
68+
// module.modulemap are system files or user files with force validation disabled,
69+
// even though the sources changed.
5770
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
5871
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
72+
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
5973
// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
6074
// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
6175
// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
6276
// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
77+
// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
78+
// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
6379
// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
6480
// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
6581
// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
6682
// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
83+
// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
84+
// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
6785

6886
// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
6987
// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
70-
// When foo.h is a user header, we will always validate it.
88+
// When foo.h is an user header, we will validate it by default.
7189
// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
7290
// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
91+
// When foo.h is an user header, we will not validate it if force validation is turned off.
92+
// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
93+
// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
7394

7495
// ===
7596
// Recompile the module if the today's date is before 01 January 2100.
7697
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
98+
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
99+
// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
77100
// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
78101
// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
102+
// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
103+
// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
104+
// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
105+
// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
79106
// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
80107
// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
108+
// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
109+
// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
110+
// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
111+
// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
81112

82113
// RUN: not diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
83114
// RUN: not diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
115+
// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
116+
// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
117+
// RUN: not diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
118+
// RUN: not diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm

0 commit comments

Comments
 (0)