-Wpragma-pack: add an additional note and fixit when warning

about unterminated push directives that are followed by a reset
('#pragma pack()')

This has been suggested by Hans Wennborg.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@309559 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d75cc20f2d76d3cfab9609c8ec9d469c19b43b07..e685311914752a581b964b90a271137dbb01bdc9 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -723,6 +723,8 @@ def warn_pragma_pack_no_pop_eof : Warning<"unterminated "
   "'#pragma pack (push, ...)' at end of file">, InGroup<PragmaPack>;
 def note_pragma_pack_here : Note<
   "previous '#pragma pack' directive that modifies alignment is here">;
+def note_pragma_pack_pop_instead_reset : Note<
+  "did you intend to use '#pragma pack (pop)' instead of '#pragma pack()'?">;
 // Follow the Microsoft implementation.
 def warn_pragma_pack_show : Warning<"value of #pragma pack(show) == %0">;
 def warn_pragma_pack_pop_identifer_and_alignment : Warning<

diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp
index f7f1f61873969a796f4b8a19e4ec1f6529274d40..4ba2a317e1f978bf2b41fab6649efee25c9af2ce 100644 (file)
--- a/lib/Sema/SemaAttr.cpp
+++ b/lib/Sema/SemaAttr.cpp
@@ -250,8 +250,22 @@ void Sema::DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind,
 void Sema::DiagnoseUnterminatedPragmaPack() {
   if (PackStack.Stack.empty())
     return;
-  for (const auto &StackSlot : llvm::reverse(PackStack.Stack))
+  bool IsInnermost = true;
+  for (const auto &StackSlot : llvm::reverse(PackStack.Stack)) {
     Diag(StackSlot.PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof);
+    // The user might have already reset the alignment, so suggest replacing
+    // the reset with a pop.
+    if (IsInnermost && PackStack.CurrentValue == PackStack.DefaultValue) {
+      DiagnosticBuilder DB = Diag(PackStack.CurrentPragmaLocation,
+                                  diag::note_pragma_pack_pop_instead_reset);
+      SourceLocation FixItLoc = Lexer::findLocationAfterToken(
+          PackStack.CurrentPragmaLocation, tok::l_paren, SourceMgr, LangOpts,
+          /*SkipTrailing=*/false);
+      if (FixItLoc.isValid())
+        DB << FixItHint::CreateInsertion(FixItLoc, "pop");
+    }
+    IsInnermost = false;
+  }
 }
 
 void Sema::ActOnPragmaMSStruct(PragmaMSStructKind Kind) { 

diff --git a/test/FixIt/fixit-pragma-pack.c b/test/FixIt/fixit-pragma-pack.c
new file mode 100644 (file)
index 0000000..acab4a8
--- /dev/null
+++ b/test/FixIt/fixit-pragma-pack.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#pragma pack (push, 1)
+#pragma pack()
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:14-[[@LINE-1]]:14}:"pop"

diff --git a/test/PCH/suspicious-pragma-pack.c b/test/PCH/suspicious-pragma-pack.c
index 01a66af34bb27266953c5bd3d3b24b7ae2735143..ac48ad3a8931ec7c3e794452b0983e9ffc495037 100644 (file)
--- a/test/PCH/suspicious-pragma-pack.c
+++ b/test/PCH/suspicious-pragma-pack.c
@@ -4,5 +4,7 @@
 #ifndef HEADER
 #define HEADER
 #pragma pack (push, 1)
+#else
+#pragma pack (2)
 #endif
-// expected-warning@-2 {{unterminated '#pragma pack (push, ...)' at end of file}}
+// expected-warning@-4 {{unterminated '#pragma pack (push, ...)' at end of file}}

diff --git a/test/Sema/pragma-pack.c b/test/Sema/pragma-pack.c
index 84a946368ff8282b9bda1caa18013d7c3d463a5e..2ed0874bfd2da8b409ada67695aad959cd7d5329 100644 (file)
--- a/test/Sema/pragma-pack.c
+++ b/test/Sema/pragma-pack.c
@@ -29,4 +29,4 @@
 // Warn about unbalanced pushes.
 #pragma pack (push,4) // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
 #pragma pack (push)   // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
-#pragma pack ()
+#pragma pack () // expected-note {{did you intend to use '#pragma pack (pop)' instead of '#pragma pack()'?}}