[clang] Rename diag notes that assumed precompiled dependencies are pch's, NFCI

[cyndyishida]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

---

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

6 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+2-2)
• (modified) clang/lib/Serialization/ASTReader.cpp (+6-6)
• (modified) clang/test/Modules/module-file-modified.c (+1)
• (modified) clang/test/Modules/validate-file-content.m (+1-1)
• (modified) clang/test/PCH/modified-module-dependency.m (+1-1)
• (modified) clang/test/PCH/validate-file-content.m (+1-1)

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 7965da593f218..a6cc87bc863c2 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -24,8 +24,8 @@ def err_fe_ast_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_ast_file_required_by : Note<"'%0' required by '%1'">;
+def note_ast_file_rebuild_required : Note<"please rebuild precompiled file '%0'">;
 def note_module_cache_path : Note<
     "after modifying system headers, please delete the module cache at '%0'">;
 
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 8d2f105b2dec3..565f01221bb06 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2862,25 +2862,25 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
       while (!ImportStack.back()->ImportedBy.empty())
         ImportStack.push_back(ImportStack.back()->ImportedBy[0]);
 
-      // The top-level PCH is stale.
-      StringRef TopLevelPCHName(ImportStack.back()->FileName);
+      // The top-level AST file is stale.
+      StringRef TopLevelASTFileName(ImportStack.back()->FileName);
       Diag(diag::err_fe_ast_file_modified)
           << *Filename << moduleKindForDiagnostic(ImportStack.back()->Kind)
-          << TopLevelPCHName << FileChange.Kind
+          << TopLevelASTFileName << FileChange.Kind
           << (FileChange.Old && FileChange.New)
           << llvm::itostr(FileChange.Old.value_or(0))
           << llvm::itostr(FileChange.New.value_or(0));
 
       // Print the import stack.
       if (ImportStack.size() > 1) {
-        Diag(diag::note_pch_required_by)
+        Diag(diag::note_ast_file_required_by)
             << *Filename << ImportStack[0]->FileName;
         for (unsigned I = 1; I < ImportStack.size(); ++I)
-          Diag(diag::note_pch_required_by)
+          Diag(diag::note_ast_file_required_by)
             << ImportStack[I-1]->FileName << ImportStack[I]->FileName;
       }
 
-      Diag(diag::note_pch_rebuild_required) << TopLevelPCHName;
+      Diag(diag::note_ast_file_rebuild_required) << TopLevelASTFileName;
     }
 
     IsOutOfDate = true;
diff --git a/clang/test/Modules/module-file-modified.c b/clang/test/Modules/module-file-modified.c
index 85018b521e288..57160f34a46cf 100644
--- a/clang/test/Modules/module-file-modified.c
+++ b/clang/test/Modules/module-file-modified.c
@@ -8,4 +8,5 @@
 #include "a.h"
 int foo = 0; // redefinition of 'foo'
 // CHECK: fatal error: file {{.*}} has been modified since the module file {{.*}} was built
+// CHECK: note: please rebuild precompiled file
 // REQUIRES: shell
diff --git a/clang/test/Modules/validate-file-content.m b/clang/test/Modules/validate-file-content.m
index 327a68a9b641e..9977aa4665f04 100644
--- a/clang/test/Modules/validate-file-content.m
+++ b/clang/test/Modules/validate-file-content.m
@@ -29,5 +29,5 @@
 // CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed
 // CHECK: '[[M_H]]' required by '[[M_PCM:.*[/\\]m.*\.pcm]]'
 // CHECK: '[[M_PCM]]' required by '[[A_PCH]]'
-// CHECK: please rebuild precompiled header '[[A_PCH]]'
+// CHECK: please rebuild precompiled file '[[A_PCH]]'
 // expected-no-diagnostics
diff --git a/clang/test/PCH/modified-module-dependency.m b/clang/test/PCH/modified-module-dependency.m
index 44c14b3a0231c..a4710dea51169 100644
--- a/clang/test/PCH/modified-module-dependency.m
+++ b/clang/test/PCH/modified-module-dependency.m
@@ -17,4 +17,4 @@
 // CHECK: file '[[TEST_H:.*[/\\]test\.h]]' has been modified since the precompiled header '[[PREFIX_PCH:.*/prefix\.pch]]' was built
 // CHECK: '[[TEST_H]]' required by '[[TEST_PCM:.*[/\\]test\.pcm]]'
 // CHECK: '[[TEST_PCM]]' required by '[[PREFIX_PCH]]'
-// CHECK: please rebuild precompiled header '[[PREFIX_PCH]]'
+// CHECK: please rebuild precompiled file '[[PREFIX_PCH]]'
diff --git a/clang/test/PCH/validate-file-content.m b/clang/test/PCH/validate-file-content.m
index aba4944a28e91..b98979341b76a 100644
--- a/clang/test/PCH/validate-file-content.m
+++ b/clang/test/PCH/validate-file-content.m
@@ -25,5 +25,5 @@
 // RUN: FileCheck %s < %t/stderr
 //
 // CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed
-// CHECK: please rebuild precompiled header '[[A_PCH]]'
+// CHECK: please rebuild precompiled file '[[A_PCH]]'
 // expected-no-diagnostics

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[vsapsai]

Right now I don't have a strong opinion about the specific terminology. Though it might change as I think about it more.

What I do have a strong opinion about is mentioning a specific file. In the first quick pass it looks like we do that already, e.g., "please rebuild precompiled file '%0'".

[vsapsai]

How natural is it to assume that "precompiled file" is the same-ish as "compiled file" aka "object file"? I'm wary of making users pay attention to such details. "Precompiled header" is an existing term, so I'm fine using it. But it is worth considering what are the differences and similarities in

• precompiled header
• compiled header
• precompiled file
• compiled file

[cyndyishida]

How natural is it to assume that "precompiled file" is the same-ish as "compiled file" aka "object file"?

I have the same concern about terminology ending up being more confusing than helpful. Personally, I've gotten used to the "pc" in pcm or pch meaning precompiled.
What do you think about the following guidelines?

1. If there's a useful distinction, or the code path naturally expects one, keep distinguishing "module file" vs "precompiled header/PCH".
2. If there's not, just use "file" and enforce that there is always a file path attached. For example, in this PR, the desired output is note: please rebuild file '/foo/bar.pcm'

[vsapsai]

I think in the second case we usually use "AST file" to lump together .pcm and .pch.

[vsapsai]

Discussed it with Cyndy more and I think that the trade-off is

• "AST file" is easier to search for as it is more specific
• "AST file" can be more confusing for non-compiler developers as they aren't required to work with and to know about AST

So even if "AST file" is technically correct (the best kind of correct), I don't think it is worth using it in a public-facing diagnostics (still fine to keep in constants like note_ast_file_required_by).

[cyndyishida]

Seems like we're ok with "precompiled file" in terms of a generalization of both precompiled headers & module files, when it's applicable, in diagnostic messages. I am going to merge this and handle aligning the rest of the diagnostics in a follow-up pr.