https://github.com/qiongsiwu created https://github.com/llvm/llvm-project/pull/209857
We have good diagnostics coverage when a pcm cannot be dropped in `ASTReader::readASTCore`, because we call `canRecoverFromOutOfDate` to check if the pcm is finalized or not. This check is missing for input file changes. This PR fixes the diagnostics, so we do not fall to the generic error `err_module_rebuild_finalized`, which is hard to reason and act on. >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] 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
