https://github.com/devajithvs updated https://github.com/llvm/llvm-project/pull/167713
>From 4402af34bc867dd67e8e5237afa8da1c2df397c6 Mon Sep 17 00:00:00 2001 From: Devajith Valaparambil Sreeramaswamy <[email protected]> Date: Tue, 11 Nov 2025 15:49:12 +0100 Subject: [PATCH] [Modules][Diagnostic] Improve diagnostics for stale module dependencies When a module becomes out of date because its dependency has changed, Clang previously reported that the dependency itself was `out of date and needs to be rebuilt`, even when the dependency had just been rebuilt. This was confusing because the real issue is that the importing module is stale due to the dependency change. This patch introduces a new diagnostic that clearly indicates which module is out of date and which dependency has changed, making it easier for users to understand what needs to be rebuilt. Before: `module file 'A.pcm' is out of date and needs to be rebuilt` (even though A.pcm was just rebuilt) After: `module file 'B.pcm' is out of date because dependency 'A.pcm' has changed` --- .../Basic/DiagnosticSerializationKinds.td | 3 ++ .../clang/Serialization/ModuleManager.h | 7 +++- clang/lib/Serialization/ASTReader.cpp | 16 ++++++- clang/lib/Serialization/ModuleManager.cpp | 6 +-- clang/test/Modules/explicit-build.cpp | 42 ++++++++++++++++++- clang/test/Modules/invalid-module-dep.c | 2 +- 6 files changed, 68 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index b80aff385e01f..d79fbcd9b8a84 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -71,6 +71,9 @@ def err_ast_file_not_found : Error< def err_ast_file_out_of_date : Error< "%select{PCH|module|precompiled}0 file '%1' is out of date and " "needs to be rebuilt%select{|: %3}2">, DefaultFatal; +def err_ast_file_dependency_out_of_date : Error< + "%select{PCH|module|AST}0 file '%1' is out of date because " + "dependency '%2' has changed%select{|: %4}3">, DefaultFatal; def err_ast_file_invalid : Error< "file '%1' is not a valid %select{PCH|module|precompiled}0 file: %2">, DefaultFatal; def note_module_file_imported_by : Note< diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 1eb74aee9787c..275cf36866294 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -198,8 +198,11 @@ class ModuleManager { /// The module file is missing. Missing, - /// The module file is out-of-date. - OutOfDate + /// The importee module file is out-of-date. + ImporteeOutOfDate, + + /// The importer file is out-of-date + ImporterOutOfDate }; using ASTFileSignatureReader = ASTFileSignature (*)(StringRef); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 634bf991b2aee..0d4481e1ceb38 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5128,7 +5128,7 @@ ASTReader::ReadASTCore(StringRef FileName, << ErrorStr; return Failure; - case ModuleManager::OutOfDate: + case ModuleManager::ImporteeOutOfDate: // We couldn't load the module file because it is out-of-date. If the // client can handle out-of-date, return it. if (ClientLoadCapabilities & ARR_OutOfDate) @@ -5139,6 +5139,20 @@ ASTReader::ReadASTCore(StringRef FileName, << moduleKindForDiagnostic(Type) << FileName << !ErrorStr.empty() << ErrorStr; return Failure; + + case ModuleManager::ImporterOutOfDate: + // We couldn't load the module file because it is out-of-date. If the + // client can handle out-of-date, return it. + if (ClientLoadCapabilities & ARR_OutOfDate) + return OutOfDate; + + // Otherwise, report that the importer is out of date because the dependency + // changed. + if (ImportedBy) + Diag(diag::err_ast_file_dependency_out_of_date) + << moduleKindForDiagnostic(ImportedBy->Kind) << ImportedBy->FileName + << FileName << !ErrorStr.empty() << StringRef(ErrorStr); + return Failure; } assert(M && "Missing module file"); diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 482c17649ed55..e53ae7f6593ad 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -125,7 +125,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, ErrorStr = IgnoreModTime ? "module file has a different size than expected" : "module file has a different size or " "modification time than expected"; - return OutOfDate; + return ImporterOutOfDate; } if (!Entry) { @@ -159,7 +159,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) { // Check the stored signature. if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) - return OutOfDate; + return ImporterOutOfDate; Module = ModuleEntry; updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); @@ -226,7 +226,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // ReadSignature unless there's something to check though. if (ExpectedSignature && checkSignature(ReadSignature(NewModule->Data), ExpectedSignature, ErrorStr)) - return OutOfDate; + return ImporterOutOfDate; // We're keeping this module. Store it everywhere. Module = Modules[*Entry] = NewModule.get(); diff --git a/clang/test/Modules/explicit-build.cpp b/clang/test/Modules/explicit-build.cpp index c2fe2024a3629..3e86c8138362f 100644 --- a/clang/test/Modules/explicit-build.cpp +++ b/clang/test/Modules/explicit-build.cpp @@ -184,6 +184,46 @@ // CHECK-NO-FILE-INDIRECT-NEXT: note: imported by module 'c' in '{{.*}}c.pcm' // CHECK-NO-FILE-INDIRECT-NOT: note: +// ------------------------------- +// Check that we diagnose stale dependencies correctly when modules change. +// +// Trigger a rebuild of A with a different configuration (-DA_EXTRA_DEFINE) to make B and C out of date +// RUN: mv %t/a.pcm %t/a-tmp.pcm +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \ +// RUN: -fmodule-name=a -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/a.pcm \ +// RUN: -DA_EXTRA_DEFINE \ +// RUN: 2>&1 | FileCheck --check-prefix=CHECK-NO-IMPLICIT-BUILD %s --allow-empty + +// Try to use C, which depends on B, which depends on the now-changed A. +// RUN: not %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \ +// RUN: -I%S/Inputs/explicit-build \ +// RUN: -fmodule-file=%t/c.pcm \ +// RUN: %s -DHAVE_A -DHAVE_B -DHAVE_C 2>&1 | FileCheck --check-prefix=CHECK-OUT-OF-DATE-INDIRECT %s +// +// CHECK-OUT-OF-DATE-INDIRECT: fatal error: module file '{{.*}}b.pcm' is out of date because dependency '{{.*}}a.pcm' has changed +// CHECK-OUT-OF-DATE-INDIRECT-NEXT: note: imported by module 'b' in '{{.*}}b.pcm' +// CHECK-OUT-OF-DATE-INDIRECT-NEXT: note: imported by module 'c' in '{{.*}}c.pcm' + +// Rebuild B with the new A, leaving C out of date. +// RUN: mv %t/b.pcm %t/b-tmp.pcm +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \ +// RUN: -fmodule-file=%t/a.pcm \ +// RUN: -fmodule-name=b -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/b.pcm \ +// RUN: -DA_EXTRA_DEFINE \ +// RUN: 2>&1 | FileCheck --check-prefix=CHECK-NO-IMPLICIT-BUILD %s --allow-empty +// +// Now only C is out of date. Try to use C. +// RUN: not %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \ +// RUN: -I%S/Inputs/explicit-build \ +// RUN: -fmodule-file=%t/c.pcm \ +// RUN: %s -DHAVE_A -DHAVE_B -DHAVE_C 2>&1 | FileCheck --check-prefix=CHECK-OUT-OF-DATE-DIRECT %s +// +// CHECK-OUT-OF-DATE-DIRECT: fatal error: module file '{{.*}}c.pcm' is out of date because dependency '{{.*}}b.pcm' has changed +// CHECK-OUT-OF-DATE-DIRECT-NOT: fatal error: module file '{{.*}}b.pcm' is out of date +// +// RUN: mv %t/a-tmp.pcm %t/a.pcm +// RUN: mv %t/b-tmp.pcm %t/b.pcm + // ------------------------------- // Check that we don't get upset if B's timestamp is newer than C's. // RUN: touch %t/b.pcm @@ -202,6 +242,6 @@ // RUN: -fmodule-file=%t/c.pcm \ // RUN: %s -DHAVE_A -DHAVE_B -DHAVE_C 2>&1 | FileCheck --check-prefix=CHECK-MISMATCHED-B %s // -// CHECK-MISMATCHED-B: fatal error: module file '{{.*}}b.pcm' is out of date and needs to be rebuilt: module file has a different size than expected +// CHECK-MISMATCHED-B: fatal error: module file '{{.*}}c.pcm' is out of date because dependency '{{.*}}b.pcm' has changed: module file has a different size than expected // CHECK-MISMATCHED-B-NEXT: note: imported by module 'c' // CHECK-MISMATCHED-B-NOT: note: diff --git a/clang/test/Modules/invalid-module-dep.c b/clang/test/Modules/invalid-module-dep.c index 2bcda8be5f1b6..70496fd83db38 100644 --- a/clang/test/Modules/invalid-module-dep.c +++ b/clang/test/Modules/invalid-module-dep.c @@ -37,7 +37,7 @@ #include <A/A.h> #include <B/B.h> // expected-warning {{conflicts with imported file}} \ // expected-note {{imported by module 'B'}} \ - // expected-error {{out of date and needs to be rebuilt}} + // expected-error {{is out of date because dependency}} //--- Sysroot/usr/include/A/module.modulemap module A [system] { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
