[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added a reviewer: aaron.ballman.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The packed attribute can still be useful in this case if the struct is
then placed inside another packed struct - the non-pod element type's
packed attribute declares that it's OK to misalign this element inside
the packed structure. (otherwise the non-pod element is not packed/its
alignment is preserved, as per D117616 
<https://reviews.llvm.org/D117616>/2771233 
<https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6>)

Fixes PR62353


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149182

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


Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -167,6 +167,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use {
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // 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*,
        S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,14 @@
           << (InBits ? 1 : 0); // (byte|bit)
     }
 
+    const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD);
+
     // Warn if we packed it unnecessarily, when the unpacked alignment is not
     // greater than the one after packing, the size in bits doesn't change and
     // the offset of each field is identical.
     if (Packed && UnpackedAlignment <= Alignment &&
-        UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+        UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+        (!CXXRD || CXXRD->isPOD()))
       Diag(D->getLocation(), diag::warn_unnecessary_packed)
           << Context.getTypeDeclType(RD);
   }



Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -167,6 +167,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use {
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // 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*,
        S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,14 @@
           << (InBits ? 1 : 0); // (byte|bit)
     }
 
+    const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD);
+
     // Warn if we packed it unnecessarily, when the unpacked alignment is not
     // greater than the one after packing, the size in bits doesn't change and
     // the offset of each field is identical.
     if (Packed && UnpackedAlignment <= Alignment &&
-        UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+        UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+        (!CXXRD || CXXRD->isPOD()))
       Diag(D->getLocation(), diag::warn_unnecessary_packed)
           << Context.getTypeDeclType(RD);
   }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits