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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
