[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko closed https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From 71796a80e15b6700fec23fbec70635aae455b188 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 34 +++
2 files changed, 42 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
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 ,
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 00..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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
@@ -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 -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test1.cpp +// RUN: cp a2.h a.h +// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test2.cpp ivanmurashko wrote: Fixed, Thanks for the hint! https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From 3c36295b5f0f1e011e11d57c5ec4e685cb917cab Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 34 +++
2 files changed, 42 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
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 ,
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 00..b8f62a79efc95c
--- /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 -fsyntax-only
-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 -fsyntax-only
-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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
@@ -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 -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test1.cpp +// RUN: cp a2.h a.h +// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test2.cpp benlangmuir wrote: The cc1 version is `-fvalidate-ast-input-files-content`. https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
@@ -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 -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test1.cpp +// RUN: cp a2.h a.h +// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test2.cpp ivanmurashko wrote: > Does this test need to use the clang driver for some reason Unfortunately, yes. The option `-fmodules-validate-input-files-content` is a driver only option. https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
@@ -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 -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test1.cpp +// RUN: cp a2.h a.h +// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang -fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap -fsyntax-only -fmodules-cache-path=%t test2.cpp benlangmuir wrote: Does this test need to use the clang driver for some reason, or can it be changed to use `%clang_cc1`, which is generally preferred for tests? https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From 48a019ead18d3e8aeb5d10d0301c2567ef1c5cd7 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 34 +++
2 files changed, 42 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index a670f2a2af41c7..d5d32931990207 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..1ead9d172f5c5e
--- /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 -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From 2a07774317c288ebe6cc55fed2fafd2989ba853e Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 34 +++
2 files changed, 42 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..1ead9d172f5c5e
--- /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 -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko edited https://github.com/llvm/llvm-project/pull/90319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From e8e9502f117d5559c4da225b0e24fe6db9c27751 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 33 +++
2 files changed, 41 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..457ad3c16e184c
--- /dev/null
+++ b/clang/test/Modules/implicit-module-no-timestamp.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: cp a1.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From ee4153f6b828c40a56b5b35d6e42cd193a1b5c31 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 33 +++
2 files changed, 41 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..457ad3c16e184c
--- /dev/null
+++ b/clang/test/Modules/implicit-module-no-timestamp.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: cp a1.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko updated
https://github.com/llvm/llvm-project/pull/90319
>From ee4153f6b828c40a56b5b35d6e42cd193a1b5c31 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 33 +++
2 files changed, 41 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..457ad3c16e184c
--- /dev/null
+++ b/clang/test/Modules/implicit-module-no-timestamp.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: cp a1.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Ivan Murashko (ivanmurashko)
Changes
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`. The patch suggests the
corresponding solution and includes a LIT test to verify it.
---
Full diff: https://github.com/llvm/llvm-project/pull/90319.diff
2 Files Affected:
- (modified) clang/lib/Serialization/ASTReader.cpp (+8)
- (added) clang/test/Modules/implicit-module-no-timestamp.cpp (+33)
``diff
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..457ad3c16e184c
--- /dev/null
+++ b/clang/test/Modules/implicit-module-no-timestamp.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: cp a1.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
``
https://github.com/llvm/llvm-project/pull/90319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] Process include files changes with -fmodules-validate-input-files-content and -fno-pch-timestamp options (PR #90319)
https://github.com/ivanmurashko created
https://github.com/llvm/llvm-project/pull/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`. The patch suggests the
corresponding solution and includes a LIT test to verify it.
>From ee4153f6b828c40a56b5b35d6e42cd193a1b5c31 Mon Sep 17 00:00:00 2001
From: Ivan Murashko
Date: Fri, 26 Apr 2024 22:45:26 +0100
Subject: [PATCH] [Modules] Process include files changes with
-fmodules-validate-input-files-content and -fno-pch-timestamp options
There are two diffs that introduce some options required when you build modules
externally and cannot rely on file modification time as a key for detecting
input file changes.
https://reviews.llvm.org/D67249 introduced the
`-fmodules-validate-input-files-content` option, which allows the use of file
content hash instead of modification time.
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.
The issue can be fixed using the same trick that was applied during the
processing of ForceCheckCXX20ModulesInputFiles. The patch suggests a solution
and includes a LIT test to verify it.
---
clang/lib/Serialization/ASTReader.cpp | 8 +
.../Modules/implicit-module-no-timestamp.cpp | 33 +++
2 files changed, 41 insertions(+)
create mode 100644 clang/test/Modules/implicit-module-no-timestamp.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0ef57a3ea804ef..4609ca3e1428c8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2630,6 +2630,14 @@ InputFile ASTReader::getInputFile(ModuleFile ,
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 00..457ad3c16e184c
--- /dev/null
+++ b/clang/test/Modules/implicit-module-no-timestamp.cpp
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: cp a1.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -fmodules-cache-path=%t test1.cpp
+// RUN: cp a2.h a.h
+// RUN: %clang -fmodules -fmodules-validate-input-files-content -Xclang
-fno-pch-timestamp -fimplicit-modules -fmodule-map-file=module.modulemap
-fsyntax-only -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
