r199446 - Improve the error message when a PCH dependency is modified

[Ben Langmuir]

Author: benlangmuir
Date: Thu Jan 16 18:19:09 2014
New Revision: 199446

URL: http://llvm.org/viewvc/llvm-project?rev=199446&view=rev
Log:
Improve the error message when a PCH dependency is modified

Show the top-level pch file as the culprit, rather than the immediate
dependency when a pch file imports a pcm from a module. To clarify the
relationship, the pch import stack is printed as notes. The old behaviour was
misleading when a pch imported a pcm (from a module), since removing the pcm
would not fix the problem, whereas rebuilding the pch would.

Added:
    cfe/trunk/test/PCH/modified-module-dependency.m
    cfe/trunk/test/PCH/modified-module-dependency.module.map
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
    cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=199446&r1=199445&r2=199446&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Thu Jan 16 18:19:09 2014
@@ -22,6 +22,8 @@ def err_fe_pch_file_modified : Error<
     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'">;
 

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=199446&r1=199445&r2=199446&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Jan 16 18:19:09 2014
@@ -1729,11 +1729,26 @@ InputFile ASTReader::getInputFile(Module
 #endif
          )) {
       if (Complain) {
-        Error(diag::err_fe_pch_file_modified, Filename, F.FileName);
-        if (Context.getLangOpts().Modules && !Diags.isDiagnosticInFlight()) {
-          Diag(diag::note_module_cache_path)
-            << PP.getHeaderSearchInfo().getModuleCachePath();
+        // 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);
+        Error(diag::err_fe_pch_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;
         }
+
+        if (!Diags.isDiagnosticInFlight())
+          Diag(diag::note_pch_rebuild_required) << TopLevelPCHName;
       }
 
       IsOutOfDate = true;

Added: cfe/trunk/test/PCH/modified-module-dependency.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/modified-module-dependency.m?rev=199446&view=auto
==============================================================================
--- cfe/trunk/test/PCH/modified-module-dependency.m (added)
+++ cfe/trunk/test/PCH/modified-module-dependency.m Thu Jan 16 18:19:09 2014
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir
+// RUN: echo '@import test;' > %t-dir/prefix.h
+// RUN: echo 'void foo(void);' > %t-dir/test.h
+// RUN: cp %S/modified-module-dependency.module.map %t-dir/module.map
+
+// Precompile prefix.pch.
+// RUN: %clang_cc1 -x objective-c -I %t-dir -fmodules -fmodules-cache-path=%t-dir/cache -emit-pch %t-dir/prefix.h -o %t-dir/prefix.pch
+
+// Modify the dependency.
+// RUN: echo ' ' >> %t-dir/test.h
+
+// Run and check the diagnostics.
+// RUN: not %clang_cc1 -x objective-c -include-pch %t-dir/prefix.pch -fmodules -fmodules-cache-path=%t-dir/cache -fsyntax-only %s 2> %t-dir/log
+// RUN: FileCheck %s < %t-dir/log
+
+// CHECK: file '{{.*}}/test.h' has been modified since the precompiled header '{{.*}}prefix.pch' was built
+// CHECK: '{{.*}}/test.h' required by '{{.*}}/test.pcm'
+// CHECK: '{{.*}}/test.pcm' required by '{{.*}}/prefix.pch'
+// CHECK: please rebuild precompiled header '{{.*}}/prefix.pch'

Added: cfe/trunk/test/PCH/modified-module-dependency.module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/modified-module-dependency.module.map?rev=199446&view=auto
==============================================================================
--- cfe/trunk/test/PCH/modified-module-dependency.module.map (added)
+++ cfe/trunk/test/PCH/modified-module-dependency.module.map Thu Jan 16 18:19:09 2014
@@ -0,0 +1,4 @@
+module test {
+  header "test.h"
+  export *
+}