[clang] Expand invalid PCM diagnostic

[NuriAmari]

Summary:

When a PCM file is loaded, it can go wrong in various ways. The current diagnostic only produces the name of the malformed PCM, not why it is malformed. Expand the diagnostic to display what went wrong!

There is only one call site for this diagnostic, and it already passes the error message:

https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L4763-L4764

Test Plan:

The modified LIT test.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Nuri Amari (NuriAmari)

Changes

Summary:

When a PCM file is loaded, it can go wrong in various ways. The current diagnostic only produces the name of the malformed PCM, not why it is malformed. Expand the diagnostic to display what went wrong!

There is only one call site for this diagnostic, and it already passes the error message:

https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L4763-L4764

Test Plan:

The modified LIT test.

---

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

2 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+1-1)
• (modified) clang/test/Modules/explicit-build.cpp (+1-1)

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index a1ae23a62802104..1c049792d16eda4 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -62,7 +62,7 @@ def err_ast_file_out_of_date : Error<
   "%select{PCH|module|AST}0 file '%1' is out of date and "
   "needs to be rebuilt%select{|: %3}2">, DefaultFatal;
 def err_ast_file_invalid : Error<
-  "file '%1' is not a valid precompiled %select{PCH|module|AST}0 file">, DefaultFatal;
+  "file '%1' is not a valid precompiled %select{PCH|module|AST}0 file: '%2'">, DefaultFatal;
 def note_module_file_imported_by : Note<
   "imported by %select{|module '%2' in }1'%0'">;
 def err_module_file_not_module : Error<
diff --git a/clang/test/Modules/explicit-build.cpp b/clang/test/Modules/explicit-build.cpp
index 16eb604708c9d83..42444369dc92c5f 100644
--- a/clang/test/Modules/explicit-build.cpp
+++ b/clang/test/Modules/explicit-build.cpp
@@ -161,7 +161,7 @@
 // RUN:            -fmodule-file=%t/not.pcm \
 // RUN:            %s 2>&1 | FileCheck --check-prefix=CHECK-BAD-FILE %s
 //
-// CHECK-BAD-FILE: fatal error: file '{{.*}}not.pcm' is not a valid precompiled module file
+// CHECK-BAD-FILE: fatal error: file '{{.*}}not.pcm' is not a valid precompiled module file: 'file too small to contain AST file magic'
 
 // RUN: not %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/nonexistent.pcm \

[dwblaikie]

Looks good.

I wonder how this happened in the first place - that the author of the diagnostic was already adding the 3rd parameter into the arguments but there was no %2 in the diagnostic text/template...

It'd be great if we had some kind of checker. I assume we fail if there are too few arguments to satisfy the %Ns, but maybe we could catch some cases and error if there are too many, too? (I guess in some cases we need it - like we pass 3 things, but depending on one of them, another is unused, etc) - @AaronBallman ?