[clang] [clang-tools-extra] [clang][Serialization] Serialize DiagStateOnPushStack to fix pragma d… (PR #190420)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Brandon (brandonxin)

<details>
<summary>Changes</summary>

…iagnostic push/pop across PCH boundary

`DiagStateOnPushStack` was not serialized in PCH files, causing `#pragma clang diagnostic pop` to emit a spurious "no matching push" warning when the corresponding push was in the preamble. This is because clangd splits files into a preamble (compiled to PCH) and the main file body, and the push/pop stack was lost during the PCH round-trip.

Serialize and deserialize DiagStateOnPushStack in `WritePragmaDiagnosticMappings`/`ReadPragmaDiagnosticMappings` so that unmatched pushes from a preamble are correctly restored.

Fixes https://github.com/clangd/clangd/issues/1167

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


5 Files Affected:

- (modified) clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp (+12) 
- (modified) clang/lib/Serialization/ASTReader.cpp (+12) 
- (modified) clang/lib/Serialization/ASTWriter.cpp (+7) 
- (added) clang/test/PCH/Inputs/pragma-diag-push.h (+1) 
- (added) clang/test/PCH/pragma-diag-push-pop-across-pch.c (+11) 


``````````diff
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 6fe48478e1175..81df26f979ac3 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1091,6 +1091,18 @@ void foo(int *x);
               NullabilityKind::NonNull);
 }
 
+TEST(DiagnosticsTest, PreamblePragmaDiagnosticPushPop) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma clang diagnostic push
+int main() {
+   return 0;
+}
+#pragma clang diagnostic pop
+)cpp");
+  auto AST = TU.build();
+  EXPECT_THAT(AST.getDiagnostics(), IsEmpty());
+}
+
 TEST(DiagnosticsTest, PreambleHeaderWithBadPragmaAssumeNonnull) {
   Annotations Header(R"cpp(
 #pragma clang assume_nonnull begin  // error-ok
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b211b0d32e1de..36d3b1123fd47 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7285,6 +7285,18 @@ void ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &Diag) {
         T[0].State = CurState;
     }
 
+    // Restore the push stack so that unmatched pushes from a preamble are
+    // visible when the main file is parsed, allowing the corresponding
+    // `#pragma diagnostic pop` to succeed.
+    assert(Idx < Record.size() &&
+           "Invalid data, missing diagnostic push stack");
+    unsigned NumPushes = Record[Idx++];
+    for (unsigned I = 0; I != NumPushes; ++I) {
+      auto *State = ReadDiagState(*FirstState, false);
+      if (!F.isModule())
+        Diag.DiagStateOnPushStack.push_back(State);
+    }
+
     // Don't try to read these mappings again.
     Record.clear();
   }
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4b3adce07f10c..f1e6ee8de1b70 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3374,6 +3374,13 @@ void ASTWriter::WritePragmaDiagnosticMappings(const DiagnosticsEngine &Diag,
   AddSourceLocation(Diag.DiagStatesByLoc.CurDiagStateLoc, Record);
   AddDiagState(Diag.DiagStatesByLoc.CurDiagState, false);
 
+  // Emit the push stack so that unmatched pushes from a preamble can be
+  // restored when the main file is parsed.  Each entry is a DiagState that
+  // was active at the time of a `#pragma diagnostic push`.
+  Record.push_back(Diag.DiagStateOnPushStack.size());
+  for (const auto *State : Diag.DiagStateOnPushStack)
+    AddDiagState(State, false);
+
   Stream.EmitRecord(DIAG_PRAGMA_MAPPINGS, Record);
 }
 
diff --git a/clang/test/PCH/Inputs/pragma-diag-push.h b/clang/test/PCH/Inputs/pragma-diag-push.h
new file mode 100644
index 0000000000000..4c1079ebd8ab9
--- /dev/null
+++ b/clang/test/PCH/Inputs/pragma-diag-push.h
@@ -0,0 +1 @@
+#pragma clang diagnostic push
diff --git a/clang/test/PCH/pragma-diag-push-pop-across-pch.c b/clang/test/PCH/pragma-diag-push-pop-across-pch.c
new file mode 100644
index 0000000000000..4fa1620700bbc
--- /dev/null
+++ b/clang/test/PCH/pragma-diag-push-pop-across-pch.c
@@ -0,0 +1,11 @@
+// Test that #pragma diagnostic push in a PCH is matched by pop in the main file.
+
+// RUN: %clang_cc1 %S/Inputs/pragma-diag-push.h -emit-pch -o %t.pch
+// RUN: %clang_cc1 %s -include-pch %t.pch -verify -fsyntax-only
+// expected-no-diagnostics
+
+#pragma clang diagnostic pop
+
+int main(void) {
+    return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/190420