[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 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

dblaikie wrote:
> aaron.ballman wrote:
> > I think this should move down to `Improvements to Clang's diagnostics` 
> > instead of adding a new category for it.
> Ah, hadn't spotted that category - thanks!
Failed to include the fix there in the initial commit, but got it in 793c5b1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

[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 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

aaron.ballman wrote:
> I think this should move down to `Improvements to Clang's diagnostics` 
> instead of adding a new category for it.
Ah, hadn't spotted that category - thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

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

[David Blaikie via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dblaikie marked an inline comment as done.
Closed by commit rGa8b0c6fa28ac: Remove -Wpacked false positive for non-pod 
types where the layout isnt… (authored by dblaikie).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp

Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for '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,19 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(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.
+// Unless the type is non-POD (for Clang ABI > 15), where the packed
+// attribute on such a type does allow the type to be packed into other
+// structures that use the packed attribute.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes `#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a nit with the release notes. Thank you for the fix!




Comment at: clang/docs/ReleaseNotes.rst:109-113
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)

I think this should move down to `Improvements to Clang's diagnostics` instead 
of adding a new category for it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

[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 marked an inline comment as done.
dblaikie added a comment.

In D149182#4312202 , @aaron.ballman 
wrote:

> Precommit CI found a valid issue (at least on Debian, the Windows failure 
> appears to be unrelated but it's really hard to tell from that output).

Hmm, right ,yeah, valid test failure - fixed.

> Also, this should have a release note, right?

Happy to add one. (done)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

[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 updated this revision to Diff 519247.
dblaikie added a comment.

Describe condition in comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp

Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for '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,19 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(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.
+// Unless the type is non-POD (for Clang ABI > 15), where the packed
+// attribute on such a type does allow the type to be packed into other
+// structures that use the packed attribute.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -106,6 +106,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes `#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 519245.
dblaikie added a comment.

Release note
Restrict change to Clang ABI 16 and above (& test that condition)
Fix AIX test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp


Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is 
unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s 
-emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is 
non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for 
the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for 
'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,16 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(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() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -106,6 +106,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` 
specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been


Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found a valid issue (at least on Debian, the Windows failure 
appears to be unrelated but it's really hard to tell from that output). Also, 
this should have a release note, right?




Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2203-2205
 // 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.

You probably should update the comment to explain the new changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

[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 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149182/new/

https://reviews.llvm.org/D149182

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

[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 
/2771233 
)

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(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(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