[clang] [clang][Modules] Adding Specific Diagnostics when a Finalized PCM is Out-of-date due to Input File Changes (PR #209857)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 16 09:09:48 PDT 2026
https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/209857
>From f7a9461740b3004bfda087ccc2ee4d92819cbc29 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Wed, 15 Jul 2026 11:43:11 -0700
Subject: [PATCH 1/3] Adding detailed diagnostics when a pcm cannot be rebuilt
because an input file is out of date.
---
clang/lib/Serialization/ASTReader.cpp | 3 +-
.../Serialization/ModuleCacheTest.cpp | 96 +++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 080cb7df04406..a72c9c95f4a22 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3300,7 +3300,8 @@ ASTReader::ReadControlBlock(ModuleFile &F,
// loaded module files, ignore missing inputs.
if (!DisableValidation && F.Kind != MK_ExplicitModule &&
F.Kind != MK_PrebuiltModule) {
- bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0;
+ bool Complain =
+ !canRecoverFromOutOfDate(F.FileName, ClientLoadCapabilities);
// If we are reading a module, we will create a verification timestamp,
// so we verify all input files. Otherwise, verify only user input
diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp
index 141e9292137a9..3034dbcd8168b 100644
--- a/clang/unittests/Serialization/ModuleCacheTest.cpp
+++ b/clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/Basic/AllDiagnostics.h"
#include "clang/Basic/FileManager.h"
#include "clang/Driver/CreateInvocationFromArgs.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -200,4 +201,99 @@ TEST_F(ModuleCacheTest, CachedModuleNewPathAllowErrors) {
ASSERT_TRUE(Diags->hasErrorOccurred());
}
+struct CapturedDiag {
+ unsigned ID;
+ std::string Message;
+};
+
+class CapturedDiagConsumer : public DiagnosticConsumer {
+ std::vector<CapturedDiag> &Diags;
+
+public:
+ CapturedDiagConsumer(std::vector<CapturedDiag> &D) : Diags(D) {}
+ void HandleDiagnostic(DiagnosticsEngine::Level Level,
+ const Diagnostic &Info) override {
+ DiagnosticConsumer::HandleDiagnostic(Level, Info);
+ SmallString<128> Msg;
+ Info.FormatDiagnostic(Msg);
+ Diags.push_back({Info.getID(), std::string(Msg)});
+ }
+};
+
+TEST_F(ModuleCacheTest, RebuildFinalizedModuleAfterInputChange) {
+ addFile("test.m", R"cpp(
+ @import M;
+ )cpp");
+ addFile("frameworks/M.framework/Headers/m.h", R"cpp(
+ void foo(void);
+ )cpp");
+ addFile("frameworks/M.framework/Modules/module.modulemap", R"cpp(
+ framework module M [system] {
+ header "m.h"
+ export *
+ }
+ )cpp");
+
+ SmallString<256> MCPArg("-fmodules-cache-path=");
+ MCPArg.append(ModuleCachePath);
+ CreateInvocationOptions CIOpts;
+ CIOpts.VFS = llvm::vfs::createPhysicalFileSystem();
+ DiagnosticOptions DiagOpts;
+ IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+ CompilerInstance::createDiagnostics(*CIOpts.VFS, DiagOpts);
+ CIOpts.Diags = Diags;
+
+ const char *Args[] = {"clang", "-fmodules", "-Fframeworks",
+ MCPArg.c_str(), "-working-directory", TestDir.c_str(),
+ "test.m"};
+
+ // Run 1: builds, loads, and finalizes M in the shared in-memory cache.
+ std::shared_ptr<CompilerInvocation> Invocation =
+ createInvocationAndEnableFree(Args, CIOpts);
+ ASSERT_TRUE(Invocation);
+ CompilerInstance Instance(std::move(Invocation));
+ Instance.setVirtualFileSystem(CIOpts.VFS);
+ Instance.setDiagnostics(Diags);
+ SyntaxOnlyAction Action;
+ ASSERT_TRUE(Instance.ExecuteAction(Action));
+ ASSERT_FALSE(Diags->hasErrorOccurred());
+
+ // Grow m.h so its recorded size no longer matches, marking M out of date on
+ // the next load. Because M is finalized in the shared cache, it can be
+ // neither dropped nor rebuilt.
+ addFile("frameworks/M.framework/Headers/m.h", R"cpp(
+ void foo(void);
+ void bar(void);
+ void baz(void);
+ )cpp");
+
+ std::vector<CapturedDiag> Captured;
+ Diags->setClient(new CapturedDiagConsumer(Captured),
+ /*ShouldOwnClient=*/true);
+
+ // Run 2 shares the module cache (and thus the finalized M).
+ std::shared_ptr<CompilerInvocation> Invocation2 =
+ createInvocationAndEnableFree(Args, CIOpts);
+ ASSERT_TRUE(Invocation2);
+ CompilerInstance Instance2(std::move(Invocation2),
+ Instance.getPCHContainerOperations(),
+ Instance.getModuleCachePtr());
+ Instance2.setVirtualFileSystem(CIOpts.VFS);
+ Instance2.setDiagnostics(Diags);
+ SyntaxOnlyAction Action2;
+ ASSERT_FALSE(Instance2.ExecuteAction(Action2));
+ ASSERT_TRUE(Diags->hasErrorOccurred());
+
+ // New notes may be added in the future, so we are checking that
+ // we have at least the expected number instead of a precise one.
+ ASSERT_GE(Captured.size(), 2u);
+ EXPECT_EQ(Captured[0].ID, (unsigned)diag::err_fe_ast_file_modified);
+ EXPECT_NE(Captured[0].Message.find("m.h"), std::string::npos);
+ EXPECT_EQ(Captured[1].ID, (unsigned)diag::note_fe_ast_file_modified);
+
+ // Make sure we do not see the generic error anywhere.
+ for (const auto &D : Captured)
+ EXPECT_NE(D.ID, (unsigned)diag::err_module_rebuild_finalized);
+}
+
} // anonymous namespace
>From 34438514547458649438980c83af32fb83576fca Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Wed, 15 Jul 2026 15:49:34 -0700
Subject: [PATCH 2/3] Address review feedback.
---
.../include/clang/Basic/DiagnosticSerializationKinds.td | 4 ++++
clang/lib/Serialization/ASTReader.cpp | 5 +++++
clang/unittests/Serialization/ModuleCacheTest.cpp | 9 ++++++---
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 5c74caf010ad9..0eecaee775402 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -23,6 +23,10 @@ def err_fe_ast_file_modified : Error<
DefaultFatal;
def note_fe_ast_file_modified : Note<
"%select{size|mtime|content}0 changed%select{| from expected %2 to %3}1">;
+def note_fe_ast_file_modified_finalized : Note<
+ "module '%0' was already finalized in this compilation and so cannot be "
+ "rebuilt; this is usually caused by an input changing after the module was "
+ "built, or a compiler bug">;
def err_fe_pch_file_overridden : Error<
"file '%0' from the precompiled header has been overridden">;
def note_ast_file_required_by : Note<"'%0' required by '%1'">;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index a72c9c95f4a22..742b58f574586 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3010,6 +3010,11 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
<< FileChange.Kind << (FileChange.Old && FileChange.New)
<< llvm::itostr(FileChange.Old.value_or(0))
<< llvm::itostr(FileChange.New.value_or(0));
+ if (getModuleManager()
+ .getModuleCache()
+ .getInMemoryModuleCache()
+ .isPCMFinal(F.FileName))
+ Diag(diag::note_fe_ast_file_modified_finalized) << F.ModuleName;
// Print the import stack.
if (ImportStack.size() > 1) {
diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp
index 3034dbcd8168b..7d5b1185be69a 100644
--- a/clang/unittests/Serialization/ModuleCacheTest.cpp
+++ b/clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -284,12 +284,15 @@ TEST_F(ModuleCacheTest, RebuildFinalizedModuleAfterInputChange) {
ASSERT_FALSE(Instance2.ExecuteAction(Action2));
ASSERT_TRUE(Diags->hasErrorOccurred());
- // New notes may be added in the future, so we are checking that
- // we have at least the expected number instead of a precise one.
- ASSERT_GE(Captured.size(), 2u);
+ ASSERT_EQ(Captured.size(), 4u);
EXPECT_EQ(Captured[0].ID, (unsigned)diag::err_fe_ast_file_modified);
EXPECT_NE(Captured[0].Message.find("m.h"), std::string::npos);
EXPECT_EQ(Captured[1].ID, (unsigned)diag::note_fe_ast_file_modified);
+ EXPECT_EQ(Captured[2].ID,
+ (unsigned)diag::note_fe_ast_file_modified_finalized);
+ EXPECT_NE(Captured[2].Message.find("'M'"), std::string::npos);
+ EXPECT_EQ(Captured[3].ID,
+ (unsigned)diag::note_ast_file_input_files_validation_status);
// Make sure we do not see the generic error anywhere.
for (const auto &D : Captured)
>From 36ff046561124456a20a6032310dba6a16fa4d2f Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Thu, 16 Jul 2026 09:09:34 -0700
Subject: [PATCH 3/3] Address code review comments.
---
clang/include/clang/Basic/DiagnosticSerializationKinds.td | 4 ++--
clang/unittests/Serialization/ModuleCacheTest.cpp | 4 ----
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 0eecaee775402..6567353346310 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -25,8 +25,8 @@ def note_fe_ast_file_modified : Note<
"%select{size|mtime|content}0 changed%select{| from expected %2 to %3}1">;
def note_fe_ast_file_modified_finalized : Note<
"module '%0' was already finalized in this compilation and so cannot be "
- "rebuilt; this is usually caused by an input changing after the module was "
- "built, or a compiler bug">;
+ "rebuilt; this is usually caused by an input changing due to "
+ "misconfigurations, or a compiler bug">;
def err_fe_pch_file_overridden : Error<
"file '%0' from the precompiled header has been overridden">;
def note_ast_file_required_by : Note<"'%0' required by '%1'">;
diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp
index 7d5b1185be69a..cadded504c296 100644
--- a/clang/unittests/Serialization/ModuleCacheTest.cpp
+++ b/clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -293,10 +293,6 @@ TEST_F(ModuleCacheTest, RebuildFinalizedModuleAfterInputChange) {
EXPECT_NE(Captured[2].Message.find("'M'"), std::string::npos);
EXPECT_EQ(Captured[3].ID,
(unsigned)diag::note_ast_file_input_files_validation_status);
-
- // Make sure we do not see the generic error anywhere.
- for (const auto &D : Captured)
- EXPECT_NE(D.ID, (unsigned)diag::err_module_rebuild_finalized);
}
} // anonymous namespace
More information about the cfe-commits
mailing list