Module: correctly set the module file kind when emitting diagnostics for file_modified

[manmanren]

Summary

I don't quite like the if statement in the patch, but given that ASTReader::Error and DelayedDiagnostic only take strings, it is hard to just change err_fe_pch_file_modified to take a %select that depends on an integer.

On the other hard, it seems impossible to have another diagnostics in flight when emitting err_fe_pch_file_modified. Is it okay to just use Diag instead of Error?

Unfortunately I was not able to come up with a test that outputs this diagnostics for a module file.

[manmanren]

Thanks Richard for the testing case. It is also obvious from the testing case that we can have another diagnostic in flight when emitting err_fe_pch_file_modified.

cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td

def err_fe_not_a_pch_file : Error<
     "input is not a PCH file: '%0'">;
 def err_fe_pch_malformed : Error<
     "malformed or corrupted AST file: '%0'">, DefaultFatal;
 def err_fe_pch_malformed_block : Error<
     "malformed block record in PCH file: '%0'">, DefaultFatal;
 def err_fe_pch_file_modified : Error<
     "file '%0' has been modified since the precompiled header '%1' was built">,
     DefaultFatal;
+def err_fe_module_file_modified : Error<
+    "file '%0' has been modified since the module file '%1' was built">,
+    DefaultFatal;
+def err_fe_ast_file_modified : Error<
+    "file '%0' has been modified since the AST file '%1' was built">,
+    DefaultFatal;
 def err_fe_pch_file_overridden : Error<
     "file '%0' from the precompiled header has been overridden">;
 def note_pch_required_by : Note<"'%0' required by '%1'">;
 def note_pch_rebuild_required : Note<"please rebuild precompiled header '%0'">;
 def note_module_cache_path : Note<
     "after modifying system headers, please delete the module cache at '%0'">;
 
 def err_pch_targetopt_mismatch : Error<

cfe/trunk/lib/Serialization/ASTReader.cpp

ASTReader::readInputFileInfo(ModuleFile &F, unsigned ID) {
   R.StoredTime = static_cast<time_t>(Record[2]);
   R.Overridden = static_cast<bool>(Record[3]);
   R.Transient = static_cast<bool>(Record[4]);
   R.Filename = Blob;
   ResolveImportedPath(F, R.Filename);
   return R;
 }
 
+static unsigned moduleKindForDiagnostic(ModuleKind Kind);
 InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
   // If this ID is bogus, just return an empty input file.
   if (ID == 0 || ID > F.InputFilesLoaded.size())
     return InputFile();
 
   // If we've already loaded this input file, return it.
   if (F.InputFilesLoaded[ID-1].getFile())
     return F.InputFilesLoaded[ID-1];
if (!Overridden && //
     if (Complain) {
       // Build a list of the PCH imports that got us here (in reverse).
       SmallVector<ModuleFile *, 4> ImportStack(1, &F);
       while (ImportStack.back()->ImportedBy.size() > 0)
         ImportStack.push_back(ImportStack.back()->ImportedBy[0]);
 
       // The top-level PCH is stale.
       StringRef TopLevelPCHName(ImportStack.back()->FileName);
+      unsigned DiagnosticKind = moduleKindForDiagnostic(ImportStack.back()->Kind);
+      if (DiagnosticKind == 0)
         Error(diag::err_fe_pch_file_modified, Filename, TopLevelPCHName);
+      else if (DiagnosticKind == 1)
+        Error(diag::err_fe_module_file_modified, Filename, TopLevelPCHName);
+      else
+        Error(diag::err_fe_ast_file_modified, Filename, TopLevelPCHName);
 
       // Print the import stack.
       if (ImportStack.size() > 1 && !Diags.isDiagnosticInFlight()) {
         Diag(diag::note_pch_required_by)
           << Filename << ImportStack[0]->FileName;
         for (unsigned I = 1; I < ImportStack.size(); ++I)
           Diag(diag::note_pch_required_by)
             << ImportStack[I-1]->FileName << ImportStack[I]->FileName;

cfe/trunk/test/Modules/module-file-modified.c

+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'int foo = 0;' > %t/a.h
+// RUN: echo 'module A { header "a.h" }' > %t/m.modulemap
+// RUN: %clang_cc1 -fmodules -emit-module -fmodule-name=A -x c %t/m.modulemap -o %t/m.pcm
+// RUN: echo 'int bar;' > %t/a.h
+// RUN: not %clang_cc1 -fmodules -fmodule-file=%t/m.pcm -fmodule-map-file=%t/m.modulemap -x c %s -I%t -fsyntax-only 2>&1 | FileCheck %s
+#include "a.h"
+int foo = 0; // redefinition of 'foo'
+// CHECK: fatal error: file {{.*}} has been modified since the module file {{.*}} was built
+// REQUIRES: shell