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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
