[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`

Author: devajithvs

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<

clang/include/clang/Serialization/ModuleManager.h

@@ -199,7 +199,10 @@ class ModuleManager {
     Missing,
 
     /// The module file is out-of-date.
-    OutOfDate
+    OutOfDate,
+
+    /// The importer file is out-of-date
+    ImporterOutOfDate
   };
 
   using ASTFileSignatureReader = ASTFileSignature (*)(StringRef);

clang/lib/Serialization/ASTReader.cpp

@@ -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");

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();

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: