[PATCH] D34114: [clang] A better format for unnecessary packed warning.

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

Richard's points are really valid here. I missed the aspect that packed always implies alignment 1, which does have an effect on the code in most of the cases listed here. I agree that the value of this warning is low, since the possibility of false-positive is quite high. Would this make for a better clang-tidy check instead (although only for cases where there is no padding and/or cases where the struct is also declared to have a stricter alignment guarantee)?



================
Comment at: test/CodeGenCXX/warn-padded-packed.cpp:19
 
-struct S4 {
-  int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+struct S4 { // expected-warning {{packed attribute is unnecessary for field: 'i'}}
+  int i;
----------------
rsmith wrote:
> This looks backwards: the packed attribute *is* necessary for field `i` here (because it reduces `i`'s alignment from 4 to 1), but not for field `c`.
Yeah, even if we wanted to suggest that aligned(1) is a better fit for these kinds of packed structs, it isn't sufficient. In this case, packed grants the 1-byte alignment, as well as ensures that arrays of this type are placed contiguously with no padding. Thus, the only place where this warning would make sense is for already packed structures that also require no padding at the end.


Repository:
  rL LLVM

https://reviews.llvm.org/D34114