[all-commits] [llvm/llvm-project] 9a9cff: [Modules] Process include files changes (#90319)

[Ivan Murashko via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 9a9cff15a15b103ae1dc1efa98b53901cdda78f1
      https://github.com/llvm/llvm-project/commit/9a9cff15a15b103ae1dc1efa98b53901cdda78f1
  Author: Ivan Murashko <ivan.murashko at gmail.com>
  Date:   2024-05-01 (Wed, 01 May 2024)

  Changed paths:
    M clang/lib/Serialization/ASTReader.cpp
    A clang/test/Modules/implicit-module-no-timestamp.cpp

  Log Message:
  -----------
  [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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications