[Modules][Diagnostic] Don't claim a METADATA mismatch is always in PCH file.

[vsapsai]

You can provide more than one AST file as an input. Emit a path for a file with a problem, so you can disambiguate between multiple files.

rdar://65005546

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Volodymyr Sapsai (vsapsai)

Changes

You can provide more than one AST file as an input. Emit a path for a file with a problem, so you can disambiguate between multiple files.

rdar://65005546

---

Full diff: https://github.com/llvm/llvm-project/pull/101280.diff

4 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+8-8)
• (modified) clang/lib/Serialization/ASTReader.cpp (+10-5)
• (modified) clang/test/Index/pch-with-errors.c (+1-1)
• (modified) clang/test/Modules/load-module-with-errors.m (+2-2)

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index eb27de5921d6a..51d0abbbec252 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -50,14 +50,14 @@ def warn_pch_vfsoverlay_mismatch : Warning<
 def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 has the following VFS overlays:\n%1">;
 def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 has no VFS overlays">;
 
-def err_pch_version_too_old : Error<
-    "PCH file uses an older PCH format that is no longer supported">;
-def err_pch_version_too_new : Error<
-    "PCH file uses a newer PCH format that cannot be read">;
-def err_pch_different_branch : Error<
-    "PCH file built from a different branch (%0) than the compiler (%1)">;
-def err_pch_with_compiler_errors : Error<
-    "PCH file contains compiler errors">;
+def err_ast_file_version_too_old : Error<
+    "%select{PCH|module|AST}0 file '%1' uses an older PCH format that is no longer supported">;
+def err_ast_file_version_too_new : Error<
+    "%select{PCH|module|AST}0 file '%1' uses a newer PCH format that cannot be read">;
+def err_ast_file_different_branch : Error<
+    "%select{PCH|module|AST}0 file '%1' built from a different branch (%2) than the compiler (%3)">;
+def err_ast_file_with_compiler_errors : Error<
+    "%select{PCH|module|AST}0 file '%1' contains compiler errors">;
 
 def err_module_file_conflict : Error<
   "module '%0' is defined in both '%1' and '%2'">, DefaultFatal;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 3cb96df12e4da..86fa96a91932f 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3023,8 +3023,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
     case METADATA: {
       if (Record[0] != VERSION_MAJOR && !DisableValidation) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
-          Diag(Record[0] < VERSION_MAJOR? diag::err_pch_version_too_old
-                                        : diag::err_pch_version_too_new);
+          Diag(Record[0] < VERSION_MAJOR ? diag::err_ast_file_version_too_old
+                                         : diag::err_ast_file_version_too_new)
+              << moduleKindForDiagnostic(F.Kind) << F.FileName;
         return VersionMismatch;
       }
 
@@ -3037,7 +3038,8 @@ ASTReader::ReadControlBlock(ModuleFile &F,
           return OutOfDate;
 
         if (!AllowASTWithCompilerErrors) {
-          Diag(diag::err_pch_with_compiler_errors);
+          Diag(diag::err_ast_file_with_compiler_errors)
+              << moduleKindForDiagnostic(F.Kind) << F.FileName;
           return HadErrors;
         }
       }
@@ -3060,7 +3062,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
       StringRef ASTBranch = Blob;
       if (StringRef(CurBranch) != ASTBranch && !DisableValidation) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
-          Diag(diag::err_pch_different_branch) << ASTBranch << CurBranch;
+          Diag(diag::err_ast_file_different_branch)
+              << moduleKindForDiagnostic(F.Kind) << F.FileName << ASTBranch
+              << CurBranch;
         return VersionMismatch;
       }
       break;
@@ -4827,7 +4831,8 @@ ASTReader::ReadASTCore(StringRef FileName,
     case AST_BLOCK_ID:
       if (!HaveReadControlBlock) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
-          Diag(diag::err_pch_version_too_old);
+          Diag(diag::err_ast_file_version_too_old)
+              << moduleKindForDiagnostic(Type) << FileName;
         return VersionMismatch;
       }
 
diff --git a/clang/test/Index/pch-with-errors.c b/clang/test/Index/pch-with-errors.c
index e8711c8e26a9b..cfe58c155cd6d 100644
--- a/clang/test/Index/pch-with-errors.c
+++ b/clang/test/Index/pch-with-errors.c
@@ -38,7 +38,7 @@ void foo(void) {
 // CHECK-INDEX: [indexEntityReference]: kind: function | name: erroneous
 
 // RUN: not %clang -fsyntax-only %s -include %t.h 2>&1 | FileCheck -check-prefix=PCH-ERR %s
-// PCH-ERR: error: PCH file contains compiler errors
+// PCH-ERR: error: PCH file '{{.*}}' contains compiler errors
 
 // RUN: not c-index-test -write-pch %t.pch foobar.c 2>&1 | FileCheck -check-prefix=NONEXISTENT %s
 // NONEXISTENT: Unable to load translation unit
diff --git a/clang/test/Modules/load-module-with-errors.m b/clang/test/Modules/load-module-with-errors.m
index 1f8e483a19e92..6e10cb3381be8 100644
--- a/clang/test/Modules/load-module-with-errors.m
+++ b/clang/test/Modules/load-module-with-errors.m
@@ -1,7 +1,7 @@
 // Note: the run lines follow their respective tests, since line/column
 // matter in this test.
 
-// pcherror-error@* {{PCH file contains compiler errors}}
+// pcherror-error-re@* {{module file '{{.*}}use_error_a.pcm' contains compiler errors}}
 @import use_error_a; // notallowerror-error {{could not build module 'use_error_a'}}
 @import use_error_b;
 // expected-no-diagnostics
@@ -61,7 +61,7 @@ void test(Error *x) {
 // RUN:   -fmodule-file=%t/prebuilt/use_error_a.pcm \
 // RUN:   -fmodule-file=%t/prebuilt/use_error_b.pcm \
 // RUN:   -fmodules-cache-path=%t 2>&1 | \
-// RUN: grep "PCH file contains compiler errors"
+// RUN: grep "module file .* contains compiler errors"
 
 // Shouldn't build the cached modules (that have errors) when not allowing
 // errors

[vsapsai]

I have more similar improvements when a diagnostic claims a mismatch is in PCH file while it is in a module file. Those would be separate PR(s) as the code change is quite different.

[jansvoboda11]

Doing the lord's work 🙏