[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

[Yan Wang via Phabricator via cfe-commits]

yawanng updated this revision to Diff 108397.

https://reviews.llvm.org/D34114

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGenCXX/warn-padded-packed.cpp


Index: test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- test/CodeGenCXX/warn-padded-packed.cpp
+++ test/CodeGenCXX/warn-padded-packed.cpp
@@ -17,7 +17,7 @@
 } __attribute__((packed));
 
 struct S4 {
-  int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+  int i;
   char c;
 } __attribute__((packed));
 
@@ -46,18 +46,18 @@
   int i; // expected-warning {{padding struct 'S8' with 3 bytes to align 'i'}}
 };
 
-struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}}
-  int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
-  int y; // expected-warning {{packed attribute is unnecessary for 'y'}}
+struct S9 {
+  int x;
+  int y;
 } __attribute__((packed));
 
-struct S10 { // expected-warning {{packed attribute is unnecessary for 'S10'}}
-  int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
+struct S10 {
+  int x;
   char a,b,c,d;
 } __attribute__((packed));
 
 
-struct S11 {
+struct S11 { // expected-warning {{packed attribute is unnecessary for 'S11'}}
   bool x;
   char a,b,c,d;
 } __attribute__((packed));
@@ -72,5 +72,28 @@
   bool b : 10;
 };
 
+struct S14 { // expected-warning {{packed attribute is unnecessary for 'S14'}}
+  char a,b,c,d;
+} __attribute__((packed));
+
+struct S15 { // expected-warning {{packed attribute is unnecessary for 'S15'}}
+  struct S14 s;
+  char a;
+} __attribute__((packed));
+
+struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundar}} expected-warning {{packed attribute is unnecessary for 'S16'}}
+  char a,b;
+} __attribute__((packed, aligned(4)));
+
+struct S17 {
+  struct S16 s;
+  char a,b;
+} __attribute__((packed, aligned(2)));
+
+struct S18 { // expected-warning {{padding size of 'S18' with 2 bytes to alignment boundar}} expected-warning {{packed attribute is unnecessary for 'S18'}}
+  struct S16 s;
+  char a,b;
+} __attribute__((packed, aligned(4)));
+
 // The warnings are emitted when the layout of the structs is computed, so we have to use them.
-void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*) { }
+void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*, S14*, S15*, S16*, S17*, S18*) { }
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1882,10 +1882,9 @@
           << (InBits ? 1 : 0); // (byte|bit)
     }
 
-    // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
-    // bother since there won't be alignment issues.
-    if (Packed && UnpackedAlignment > CharUnits::One() && 
-        getSize() == UnpackedSize)
+    // Warn if we packed it unnecessarily, when the unpacked alignment is not
+    // greater than the one after packing.
+    if (Packed && UnpackedAlignment <= Alignment)
       Diag(D->getLocation(), diag::warn_unnecessary_packed)
           << Context.getTypeDeclType(RD);
   }
@@ -1978,12 +1977,6 @@
           << PadSize
           << (InBits ? 1 : 0); // (byte|bit)
   }
-
-  // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
-  // bother since there won't be alignment issues.
-  if (isPacked && UnpackedAlign > CharBitNum && Offset == UnpackedOffset)
-    Diag(D->getLocation(), diag::warn_unnecessary_packed)
-        << D->getIdentifier();
 }
 
 static const CXXMethodDecl *computeKeyFunction(ASTContext &Context,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34114.108397.patch
Type: text/x-patch
Size: 3512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170727/d171d627/attachment.bin>