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

Chih-Hung Hsieh via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 27 14:45:56 PDT 2017


chh added inline comments.


================
Comment at: lib/AST/RecordLayoutBuilder.cpp:1887
+    // greater than the one after packing.
+    if (Packed && UnpackedAlignment <= Alignment)
       Diag(D->getLocation(), diag::warn_unnecessary_packed)
----------------
With this change, UnpackedSize is unused and caused a warning about unused-but-set-variable. Please use it or remove the variable.

I think UnpackedSizeInBits should be used somehow in the condition, because bit fields can be affected by the packed attribute. Please add a few unit test cases with bit fields. For example, the following cases showed that S21 is affected by packed, but got a wrong "unnecessary" warning after this change.


```
struct S21 {
  unsigned char a:6;
  unsigned char b:6;
} __attribute__((packed, aligned(1)));
struct S22 {
  unsigned char a:6;
  unsigned char b:6;
} __attribute__((aligned(1)));
struct S23 {
  unsigned char a:6;
  unsigned char b:6;
};

```
Warnings:

```
  padding size of 'S21' with 4 bits to alignment boundary
  packed attribute is unnecessary for 'S21'
  padding struct 'S22' with 2 bits to align 'b'
  padding size of 'S22' with 2 bits to alignment boundary
  padding struct 'S23' with 2 bits to align 'b'
  padding size of 'S23' with 2 bits to alignment boundary

```



https://reviews.llvm.org/D34114





More information about the cfe-commits mailing list