Skip to content

Conversation

jansvoboda11
Copy link
Contributor

#137363 was supposed to be NFC for the CrossProcessModuleCache (a.k.a normal implicit module builds), but accidentally passed the wrong path to sys::fs::status. Then, #141358 removed the correct path that should've been passed instead. (The variable was flagged as unused.) None of our existing tests caught this regression, we only found out due to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang for PCM input file validation, and uses it to create more reliable tests of the -fmodules-validate-once-per-build-session flag.

(cherry picked from commit ce8abef)

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Oct 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 13, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

#137363 was supposed to be NFC for the CrossProcessModuleCache (a.k.a normal implicit module builds), but accidentally passed the wrong path to sys::fs::status. Then, #141358 removed the correct path that should've been passed instead. (The variable was flagged as unused.) None of our existing tests caught this regression, we only found out due to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang for PCM input file validation, and uses it to create more reliable tests of the -fmodules-validate-once-per-build-session flag.

(cherry picked from commit ce8abef)


Patch is 20.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163264.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+1)
  • (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+4)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+4)
  • (modified) clang/lib/Serialization/ModuleCache.cpp (+3-1)
  • (modified) clang/test/Modules/fmodules-validate-once-per-build-session.c (+125-110)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index c28a919e35d08..76f9addab47d8 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -624,6 +624,7 @@ def MissingFieldInitializers : DiagGroup<"missing-field-initializers",
 def ModuleLock : DiagGroup<"module-lock">;
 def ModuleBuild : DiagGroup<"module-build">;
 def ModuleImport : DiagGroup<"module-import">;
+def ModuleValidation : DiagGroup<"module-validation">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
 def ModuleIncludeDirectiveTranslation : DiagGroup<"module-include-translation">;
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 584c8d62280bf..6494f3415b7a5 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -82,6 +82,10 @@ def remark_module_import : Remark<
   "importing module '%0'%select{| into '%3'}2 from '%1'">,
   ShowInSystemHeader,
   InGroup<ModuleImport>;
+def remark_module_validation : Remark<
+  "validating %0 input files in module '%1' from '%2'">,
+  ShowInSystemHeader,
+  InGroup<ModuleValidation>;
 
 def err_imported_module_not_found : Error<
     "module '%0' in precompiled file '%1' %select{(imported by precompiled file '%2') |}4"
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 30e0973149594..3e7ccfdd9db5d 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3103,6 +3103,10 @@ ASTReader::ReadControlBlock(ModuleFile &F,
             F.Kind == MK_ImplicitModule)
           N = ForceValidateUserInputs ? NumUserInputs : 0;
 
+        if (N != 0)
+          Diag(diag::remark_module_validation)
+              << N << F.ModuleName << F.FileName;
+
         for (unsigned I = 0; I < N; ++I) {
           InputFile IF = getInputFile(F, I+1, Complain);
           if (!IF.getFile() || IF.isOutOfDate())
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index f42bdc16d815d..88ad8dd6495dd 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -34,8 +34,10 @@ class CrossProcessModuleCache : public ModuleCache {
   }
 
   std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
+    std::string TimestampFilename =
+        serialization::ModuleFile::getTimestampFilename(ModuleFilename);
     llvm::sys::fs::file_status Status;
-    if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
+    if (llvm::sys::fs::status(TimestampFilename, Status) != std::error_code{})
       return 0;
     return llvm::sys::toTimeT(Status.getLastModificationTime());
   }
diff --git a/clang/test/Modules/fmodules-validate-once-per-build-session.c b/clang/test/Modules/fmodules-validate-once-per-build-session.c
index d9d79b001e30c..2348ca1381e8a 100644
--- a/clang/test/Modules/fmodules-validate-once-per-build-session.c
+++ b/clang/test/Modules/fmodules-validate-once-per-build-session.c
@@ -1,119 +1,134 @@
-#include "foo.h"
-#include "bar.h"
-
-// Clear the module cache.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t/Inputs
-// RUN: mkdir -p %t/modules-to-compare
+// This tests the behavior of -fmodules-validate-once-per-build-session with
+// different combinations of flags and states of the module cache.
 
-// ===
-// Create a module.  We will use -I or -isystem to determine whether to treat
-// foo.h as a system header.
-// RUN: echo 'void meow(void);' > %t/Inputs/foo.h
-// RUN: echo 'void woof(void);' > %t/Inputs/bar.h
-// RUN: echo 'module Foo { header "foo.h" }' > %t/Inputs/module.modulemap
-// RUN: echo 'extern module Bar "bar.modulemap"' >> %t/Inputs/module.modulemap
-// RUN: echo 'module Bar { header "bar.h" }' > %t/Inputs/bar.modulemap
+// Note: The `sleep 1` commands sprinkled throughout this test make the strict
+//       comparisons of epoch mtimes work as expected. Some may be unnecessary,
+//       but make the intent clearer.
 
-// ===
-// Compile the module.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-before.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-before.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-before-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-before-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-before-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-before-user-no-force.pcm
-
-// ===
-// Use it, and make sure that we did not recompile it.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-use-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: echo "-fsyntax-only -fmodules -fmodules-cache-path=%/t/module-cache" > %t/ctx.rsp
+// RUN: echo "-fbuild-session-file=%/t/module-cache/session.timestamp"      >> %t/ctx.rsp
+// RUN: echo "-fmodules-validate-once-per-build-session"                    >> %t/ctx.rsp
+// RUN: echo "-Rmodule-build -Rmodule-validation"                           >> %t/ctx.rsp
 
-// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- include/foo.h
+//--- include/module.modulemap
+module Foo { header "foo.h" }
 
-// ===
-// Change the sources.
+//--- clean.c
+// Clean module cache. Modules will get compiled regardless of validation settings.
+// RUN: mkdir %t/module-cache
 // RUN: sleep 1
-// RUN: echo 'void meow2(void);' > %t/Inputs/foo.h
-// RUN: echo 'module Bar { header "bar.h" export * }' > %t/Inputs/bar.modulemap
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+// RUN: %clang @%t/ctx.rsp %t/clean.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/clean.c
+#include "foo.h"
+// CHECK: building module 'Foo'
 
-// ===
-// Use the module, and make sure that we did not recompile it if foo.h or
-// module.modulemap are system files or user files with force validation disabled,
-// even though the sources changed.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=1390000000 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- no-change-same-session.c
+// Populated module cache in the same build session with unchanged inputs.
+// Validation only happens when it's forced for user headers. No compiles.
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c  --check-prefix=CHECK-VALIDATION-ONLY
+// RUN: %clang @%t/ctx.rsp %t/no-change-same-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+#include "foo.h"
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: building module 'Foo'
+// CHECK-VALIDATION-ONLY: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-VALIDATION-ONLY-NOT: building module 'Foo'
 
-// RUN: diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// When foo.h is an user header, we will validate it by default.
-// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// When foo.h is an user header, we will not validate it if force validation is turned off.
-// RUN: diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- change-same-session.c
+// Populated module cache in the same build session with changed inputs.
+// Validation only happens when it's forced for user headers and results in compilation.
+// RUN: sleep 1
+// RUN: touch %t/include/foo.h
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-VALIDATION-AND-BUILD
+// RUN: %clang @%t/ctx.rsp %t/change-same-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/change-same-session.c --check-prefix=CHECK-NO-VALIDATION-OR-BUILD --allow-empty
+#include "foo.h"
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-NO-VALIDATION-OR-BUILD-NOT: building module 'Foo'
+// CHECK-VALIDATION-AND-BUILD: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-VALIDATION-AND-BUILD: building module 'Foo'
 
-// ===
-// Recompile the module if the today's date is before 01 January 2100.
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache -fsyntax-only -isystem %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user -fsyntax-only -I %t/Inputs -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: %clang_cc1 -cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/modules-cache-user-no-force -fsyntax-only -I %t/Inputs -fno-modules-force-validate-user-headers -fmodules-validate-system-headers -fbuild-session-timestamp=4102441200 -fmodules-validate-once-per-build-session %s
-// RUN: ls -R %t/modules-cache | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user | grep Bar.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Foo.pcm.timestamp
-// RUN: ls -R %t/modules-cache-user-no-force | grep Bar.pcm.timestamp
-// RUN: cp %t/modules-cache/Foo.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: cp %t/modules-cache/Bar.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: cp %t/modules-cache-user/Foo.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: cp %t/modules-cache-user/Bar.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: cp %t/modules-cache-user-no-force/Foo.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: cp %t/modules-cache-user-no-force/Bar.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- change-new-session.c
+// Populated module cache in a new build session with changed inputs.
+// All configurations validate and recompile.
+// RUN: sleep 1
+// RUN: touch %t/include/foo.h
+// RUN: sleep 1
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-VALIDATE-ONCE
+// NOTE: Forced user headers validation causes redundant validation of the just-built module.
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-FORCE-VALIDATE-TWICE
+// RUN: %clang @%t/ctx.rsp %t/change-new-session.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/change-new-session.c --check-prefixes=CHECK,CHECK-VALIDATE-ONCE
+#include "foo.h"
+// CHECK: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK: building module 'Foo'
+// CHECK-VALIDATE-ONCE-NOT: validating {{[0-9]+}} input files in module 'Foo'
+// CHECK-FORCE-VALIDATE-TWICE: validating {{[0-9]+}} input files in module 'Foo'
 
-// RUN: not diff %t/modules-to-compare/Foo-before.pcm %t/modules-to-compare/Foo-after.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before.pcm %t/modules-to-compare/Bar-after.pcm
-// RUN: not diff %t/modules-to-compare/Foo-before-user.pcm %t/modules-to-compare/Foo-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user.pcm %t/modules-to-compare/Bar-after-user.pcm
-// RUN: not diff %t/modules-to-compare/Foo-before-user-no-force.pcm %t/modules-to-compare/Foo-after-user-no-force.pcm
-// RUN: not diff %t/modules-to-compare/Bar-before-user-no-force.pcm %t/modules-to-compare/Bar-after-user-no-force.pcm
+//--- no-change-new-session-twice.c
+// Populated module cache in a new build session with unchanged inputs.
+// At first, all configurations validate but don't recompile.
+// RUN: sleep 1
+// RUN: touch %t/module-cache/session.timestamp
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=2 \
+// RUN:   -I %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=3 \
+// RUN:   -I %t/include -fmodules-validate-system-headers -Xclang -fno-modules-force-validate-user-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-ONCE
+//
+// Then, only the forced user header validation performs redundant validation (but no compilation).
+// All other configurations do not validate and do not compile.
+// RUN: sleep 1
+// RUN: %clang @%t/ctx.rsp %t/no-change-new-session-twice.c -DCTX=1 \
+// RUN:   -isystem %t/include -fmodules-validate-system-headers \
+// RUN:     2>&1 | FileCheck %t/no-change-new-session-twice.c --check-prefix=CHECK-NOT-TWICE --allow-empty
+// NOTE: Forced user headers validation causes redundant validation of the just-validated module.
+// RUN: %clang @%t/ctx.rsp %t/no...
[truncated]

@c-rhodes
Copy link
Collaborator

Please can one of the Clang maintainers take a look at this and give me their thoughts on merging this into the release branch?

@c-rhodes c-rhodes moved this from Needs Triage to Needs Review in LLVM Release Status Oct 15, 2025
@vgvassilev
Copy link
Contributor

This looks reasonable to me.

@c-rhodes
Copy link
Collaborator

This looks reasonable to me.

ok thanks, please approve and I'll land.

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-project-automation github-project-automation bot moved this from Needs Review to Needs Merge in LLVM Release Status Oct 16, 2025
…m#162965)

llvm#137363 was supposed to be NFC for the `CrossProcessModuleCache` (a.k.a
normal implicit module builds), but accidentally passed the wrong path
to `sys::fs::status`. Then, llvm#141358 removed the correct path that
should've been passed instead. (The variable was flagged as unused.)
None of our existing tests caught this regression, we only found out due
to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang
for PCM input file validation, and uses it to create more reliable tests
of the `-fmodules-validate-once-per-build-session` flag.

(cherry picked from commit ce8abef)
@c-rhodes c-rhodes force-pushed the once-per-build-session-release-21-x branch from 33ce18e to dfdee9a Compare October 16, 2025 08:07
@c-rhodes c-rhodes merged commit dfdee9a into llvm:release/21.x Oct 16, 2025
1 check was pending
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Oct 16, 2025
Copy link

@jansvoboda11 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@jansvoboda11 jansvoboda11 deleted the once-per-build-session-release-21-x branch October 16, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants