https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/69975
>From 0270c76e779457486ee89f100db2b7151832c290 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Tue, 10 Oct 2023 14:16:13 -0700 Subject: [PATCH 1/4] [clang][modules] Make `DIAGNOSTIC_OPTIONS` skippable --- clang/include/clang/Driver/Options.td | 4 ++ clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++- clang/lib/Serialization/ASTWriter.cpp | 4 ++ .../Modules/diagnostic-options-mismatch.c | 41 +++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 clang/test/Modules/diagnostic-options-mismatch.c diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 5415b18d3f406df..68ec32bdf56a555 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2944,6 +2944,10 @@ def fno_modules_validate_textual_header_includes : MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>, HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. " "This flag will be removed in a future Clang release.">; +defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options", + HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse, + PosFlag<SetTrue, [], [], "Disable writing diagnostic options">, + NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>; def fincremental_extensions : Flag<["-"], "fincremental-extensions">, diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index c7d95006bb779ad..d6dd94fe9466299 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -219,6 +219,9 @@ class HeaderSearchOptions { unsigned ModulesValidateDiagnosticOptions : 1; + /// Whether to entirely skip writing diagnostic options. + unsigned ModulesSkipDiagnosticOptions : 1; + unsigned ModulesHashContent : 1; /// Whether we should include all things that could impact the module in the @@ -238,7 +241,8 @@ class HeaderSearchOptions { ModulesValidateSystemHeaders(false), ValidateASTInputFilesContent(false), ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), - ModulesValidateDiagnosticOptions(true), ModulesHashContent(false), + ModulesValidateDiagnosticOptions(true), + ModulesSkipDiagnosticOptions(false), ModulesHashContent(false), ModulesStrictContextHash(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 27700c711d52fdd..e063f8d6acc295a 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1215,6 +1215,9 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // Diagnostic options. const auto &Diags = Context.getDiagnostics(); const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions(); + if (!PP.getHeaderSearchInfo() + .getHeaderSearchOpts() + .ModulesSkipDiagnosticOptions) { #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name); #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast<unsigned>(DiagOpts.get##Name())); @@ -1229,6 +1232,7 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // are generally transient files and will almost always be overridden. Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record); Record.clear(); + } // Header search paths. Record.clear(); diff --git a/clang/test/Modules/diagnostic-options-mismatch.c b/clang/test/Modules/diagnostic-options-mismatch.c new file mode 100644 index 000000000000000..cdd6f897729a9d8 --- /dev/null +++ b/clang/test/Modules/diagnostic-options-mismatch.c @@ -0,0 +1,41 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: split-file %s %t + +//--- module.modulemap +module Mod { header "mod.h" } +//--- mod.h +//--- tu.c +#include "mod.h" + +// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REBUILD +// DID-REBUILD: remark: building module 'Mod' + +// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty +// DID-REUSE-NOT: remark: building module 'Mod' +// +// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-DIAG-OPTS +// NO-DIAG-OPTS-NOT: Diagnostic flags: + +// When disabling validation of diagnostic options, mismatches are not checked for, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Wnon-modular-include-in-module +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty +// +// RUN: %clang_cc1 -module-file-info %t/cache3/Mod.pcm | FileCheck %s --check-prefix=OLD-DIAG-OPTS +// OLD-DIAG-OPTS: Diagnostic flags: +// OLD-DIAG-OPTS-NEXT: -Wnon-modular-include-in-module >From b4bdbaf98b0fd2308d38a9334fa045a88885a115 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Tue, 10 Oct 2023 14:18:33 -0700 Subject: [PATCH 2/4] [clang][modules] Make `HEADER_SEARCH_PATHS` skippable --- clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++- clang/lib/Serialization/ASTWriter.cpp | 52 +++++++++---------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index d6dd94fe9466299..766b1919a5b5eab 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -222,6 +222,9 @@ class HeaderSearchOptions { /// Whether to entirely skip writing diagnostic options. unsigned ModulesSkipDiagnosticOptions : 1; + /// Whether to entirely skip writing header search paths. + unsigned ModulesSkipHeaderSearchPaths : 1; + unsigned ModulesHashContent : 1; /// Whether we should include all things that could impact the module in the @@ -242,7 +245,8 @@ class HeaderSearchOptions { ValidateASTInputFilesContent(false), ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), ModulesValidateDiagnosticOptions(true), - ModulesSkipDiagnosticOptions(false), ModulesHashContent(false), + ModulesSkipDiagnosticOptions(false), + ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false), ModulesStrictContextHash(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 e063f8d6acc295a..d5689cb3c97cb39 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1212,12 +1212,12 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, Record.clear(); } + const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); + // Diagnostic options. const auto &Diags = Context.getDiagnostics(); const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions(); - if (!PP.getHeaderSearchInfo() - .getHeaderSearchOpts() - .ModulesSkipDiagnosticOptions) { + if (!HSOpts.ModulesSkipDiagnosticOptions) { #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name); #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast<unsigned>(DiagOpts.get##Name())); @@ -1235,33 +1235,31 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, } // Header search paths. - Record.clear(); - const HeaderSearchOptions &HSOpts = - PP.getHeaderSearchInfo().getHeaderSearchOpts(); - - // Include entries. - Record.push_back(HSOpts.UserEntries.size()); - for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) { - const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I]; - AddString(Entry.Path, Record); - Record.push_back(static_cast<unsigned>(Entry.Group)); - Record.push_back(Entry.IsFramework); - Record.push_back(Entry.IgnoreSysRoot); - } + if (!HSOpts.ModulesSkipHeaderSearchPaths) { + // Include entries. + Record.push_back(HSOpts.UserEntries.size()); + for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) { + const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I]; + AddString(Entry.Path, Record); + Record.push_back(static_cast<unsigned>(Entry.Group)); + Record.push_back(Entry.IsFramework); + Record.push_back(Entry.IgnoreSysRoot); + } - // System header prefixes. - Record.push_back(HSOpts.SystemHeaderPrefixes.size()); - for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) { - AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record); - Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader); - } + // System header prefixes. + Record.push_back(HSOpts.SystemHeaderPrefixes.size()); + for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) { + AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record); + Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader); + } - // VFS overlay files. - Record.push_back(HSOpts.VFSOverlayFiles.size()); - for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles) - AddString(VFSOverlayFile, Record); + // VFS overlay files. + Record.push_back(HSOpts.VFSOverlayFiles.size()); + for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles) + AddString(VFSOverlayFile, Record); - Stream.EmitRecord(HEADER_SEARCH_PATHS, Record); + Stream.EmitRecord(HEADER_SEARCH_PATHS, Record); + } // Write out the diagnostic/pragma mappings. WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule); >From f602cd00b56e20861f5d8800bfcc6245a7d3d37e Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Tue, 10 Oct 2023 14:21:28 -0700 Subject: [PATCH 3/4] [clang][deps] Skip `DIAGNOSTIC_OPTIONS` and `HEADER_SEARCH_PATHS` --- .../lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 29df0c3a0afdb5c..dc7bb289cf8f905 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -252,6 +252,8 @@ class DependencyScanningAction : public tooling::ToolAction { // TODO: Implement diagnostic bucketing to reduce the impact of strict // context hashing. ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true; + ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true; + ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true; // Avoid some checks and module map parsing when loading PCM files. ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; >From f33ab359c2345c7571f62ccccac4a7af0d86b6ed Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Wed, 25 Oct 2023 15:30:42 -0700 Subject: [PATCH 4/4] Add -fmodules-skip-header-search-paths test, run clang-format --- clang/include/clang/Driver/Options.td | 4 +++ clang/lib/Frontend/FrontendActions.cpp | 15 ++++++++ clang/lib/Serialization/ASTWriter.cpp | 20 +++++------ .../Modules/header-search-paths-mismatch.c | 36 +++++++++++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 clang/test/Modules/header-search-paths-mismatch.c diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 68ec32bdf56a555..2a37c089fa44f40 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2948,6 +2948,10 @@ defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-opti HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse, PosFlag<SetTrue, [], [], "Disable writing diagnostic options">, NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>; +defm modules_skip_header_search_paths : BoolFOption<"modules-skip-header-search-paths", + HeaderSearchOpts<"ModulesSkipHeaderSearchPaths">, DefaultFalse, + PosFlag<SetTrue, [], [], "Disable writing header search paths">, + NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>; def fincremental_extensions : Flag<["-"], "fincremental-extensions">, diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index ce6e0b7126b635a..2afcf1cf9f68c81 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -661,6 +661,21 @@ namespace { return false; } + bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts, + bool Complain) override { + Out.indent(2) << "Header search paths:\n"; + Out.indent(4) << "User entries:\n"; + for (const auto &Entry : HSOpts.UserEntries) + Out.indent(6) << Entry.Path << "\n"; + Out.indent(4) << "System header prefixes:\n"; + for (const auto &Prefix : HSOpts.SystemHeaderPrefixes) + Out.indent(6) << Prefix.Prefix << "\n"; + Out.indent(4) << "VFS overlay files:\n"; + for (const auto &Overlay : HSOpts.VFSOverlayFiles) + Out.indent(6) << Overlay << "\n"; + return false; + } + bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain, std::string &SuggestedPredefines) override { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index d5689cb3c97cb39..b5a9f745b5da86f 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1222,16 +1222,16 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast<unsigned>(DiagOpts.get##Name())); #include "clang/Basic/DiagnosticOptions.def" - Record.push_back(DiagOpts.Warnings.size()); - for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I) - AddString(DiagOpts.Warnings[I], Record); - Record.push_back(DiagOpts.Remarks.size()); - for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I) - AddString(DiagOpts.Remarks[I], Record); - // Note: we don't serialize the log or serialization file names, because they - // are generally transient files and will almost always be overridden. - Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record); - Record.clear(); + Record.push_back(DiagOpts.Warnings.size()); + for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I) + AddString(DiagOpts.Warnings[I], Record); + Record.push_back(DiagOpts.Remarks.size()); + for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I) + AddString(DiagOpts.Remarks[I], Record); + // Note: we don't serialize the log or serialization file names, because + // they are generally transient files and will almost always be overridden. + Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record); + Record.clear(); } // Header search paths. diff --git a/clang/test/Modules/header-search-paths-mismatch.c b/clang/test/Modules/header-search-paths-mismatch.c new file mode 100644 index 000000000000000..d733b39bb2367fa --- /dev/null +++ b/clang/test/Modules/header-search-paths-mismatch.c @@ -0,0 +1,36 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: split-file %s %t + +//--- module.modulemap +module Mod { header "mod.h" } +//--- mod.h +//--- tu.c +#include "mod.h" + +//--- one/foo.h +//--- two/foo.h + +// By default, mismatched header search paths are ignored, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -I %t/one +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -I %t/two -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --allow-empty --check-prefix=DID-REUSE +// DID-REUSE-NOT: remark: building module 'Mod' +// +// RUN: %clang_cc1 -module-file-info %t/cache1/Mod.pcm | FileCheck %s --check-prefix=HS-PATHS -DPREFIX=%/t +// HS-PATHS: Header search paths: +// HS-PATHS-NEXT: User entries: +// HS-PATHS-NEXT: [[PREFIX]]/one + +// When skipping serialization of header search paths, mismatches cannot be detected, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/one +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/two -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty +// +// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-HS-PATHS +// NO-HS-PATHS-NOT: Header search paths: _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits