[clang] 9a9cff1 - [Modules] Process include files changes (#90319)

[via cfe-commits]

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