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

[Yan Wang via Phabricator via cfe-commits]

yawanng updated this revision to Diff 108395.
yawanng edited the summary of this revision.
yawanng added a comment.

Including cases '__attribute__((packed, aligned(X)));'. When the alignment before packing is less or equal to the packed one, he 'packed' attribute not including 'aligned(X)' seems unnecessary.  Because the alignment won't change as well as the size after removing 'packed'.


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.108395.patch
Type: text/x-patch
Size: 3516 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170727/5014b610/attachment-0001.bin>