[clang][Modules] Adding Specific Diagnostics when a Finalized PCM is Out-of-date due to Input File Changes (#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.

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 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'">;

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) {
@@ -3300,7 +3305,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

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,98 @@ 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());
+
+  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);
+}
+
 } // anonymous namespace