Author: Ivan Murashko Date: 2024-05-01T09:07:57+01:00 New Revision: 9a9cff15a15b103ae1dc1efa98b53901cdda78f1
URL: https://github.com/llvm/llvm-project/commit/9a9cff15a15b103ae1dc1efa98b53901cdda78f1 DIFF: https://github.com/llvm/llvm-project/commit/9a9cff15a15b103ae1dc1efa98b53901cdda78f1.diff LOG: [Modules] Process include files changes (#90319) There were two diffs that introduced some options useful when you build modules externally and cannot rely on file modification time as the key for detecting input file changes: - [D67249](https://reviews.llvm.org/D67249) introduced the `-fmodules-validate-input-files-content` option, which allows the use of file content hash in addition to the modification time. - [D141632](https://reviews.llvm.org/D141632) propagated the use of `-fno-pch-timestamps` with Clang modules. There is a problem when the size of the input file (header) is not modified but the content is. In this case, Clang cannot detect the file change when the `-fno-pch-timestamps` option is used. The `-fmodules-validate-input-files-content` option should help, but there is an issue with its application: it's not applied when the modification time is stored as zero that is the case for `-fno-pch-timestamps`. The issue can be fixed using the same trick that was applied during the processing of `ForceCheckCXX20ModulesInputFiles`: ``` // When ForceCheckCXX20ModulesInputFiles and ValidateASTInputFilesContent // enabled, it is better to check the contents of the inputs. Since we can't // get correct modified time information for inputs from overriden inputs. if (HSOpts.ForceCheckCXX20ModulesInputFiles && ValidateASTInputFilesContent && F.StandardCXXModule && FileChange.Kind == Change::None) FileChange = HasInputContentChanged(FileChange); ``` The patch suggests the solution similar to the presented above and includes a LIT test to verify it. Added: clang/test/Modules/implicit-module-no-timestamp.cpp Modified: clang/lib/Serialization/ASTReader.cpp Removed: ################################################################################ diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 143fbc7feb3ab7..29b81c1a753cf5 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) { F.StandardCXXModule && FileChange.Kind == Change::None) FileChange = HasInputContentChanged(FileChange); + // When we have StoredTime equal to zero and ValidateASTInputFilesContent, + // it is better to check the content of the input files because we cannot rely + // on the file modification time, which will be the same (zero) for these + // files. + if (!StoredTime && ValidateASTInputFilesContent && + FileChange.Kind == Change::None) + FileChange = HasInputContentChanged(FileChange); + // For an overridden file, there is nothing to validate. if (!Overridden && FileChange.Kind != Change::None) { if (Complain && !Diags.isDiagnosticInFlight()) { diff --git a/clang/test/Modules/implicit-module-no-timestamp.cpp b/clang/test/Modules/implicit-module-no-timestamp.cpp new file mode 100644 index 00000000000000..1b681a610bab22 --- /dev/null +++ b/clang/test/Modules/implicit-module-no-timestamp.cpp @@ -0,0 +1,34 @@ +// UNSUPPORTED: system-windows +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: cd %t +// +// RUN: cp a1.h a.h +// RUN: %clang_cc1 -fmodules -fvalidate-ast-input-files-content -fno-pch-timestamp -fmodule-map-file=module.modulemap -fmodules-cache-path=%t test1.cpp +// RUN: cp a2.h a.h +// RUN: %clang_cc1 -fmodules -fvalidate-ast-input-files-content -fno-pch-timestamp -fmodule-map-file=module.modulemap -fmodules-cache-path=%t test2.cpp + +//--- a1.h +#define FOO + +//--- a2.h +#define BAR + +//--- module.modulemap +module a { + header "a.h" +} + +//--- test1.cpp +#include "a.h" + +#ifndef FOO +#error foo +#endif + +//--- test2.cpp +#include "a.h" + +#ifndef BAR +#error bar +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits