Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Alexey Bataev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL254596: PR25575: Make GCC 4.4+ comatible layout for packed 
bit-fileds of char type… (authored by ABataev).

Changed prior to commit:
  http://reviews.llvm.org/D14872?vs=41648=41724#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D14872

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/Sema/struct-packed-align.c

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2784,9 +2784,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
-def warn_attribute_ignored_for_field_of_type : Warning<
-  "%0 attribute ignored for field of type %1">,
-  InGroup;
+def warn_attribute_packed_for_bitfield : Warning<
+  "'packed' attribute was ignored on bit-fields with single-byte alignment "
+  "in older versions of GCC and Clang">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<
   "%select{alignment|size}0 of field %1 (%2 bits) does not match the "
   "%select{alignment|size}0 of the first field in transparent union; "
Index: cfe/trunk/test/Sema/struct-packed-align.c
===
--- cfe/trunk/test/Sema/struct-packed-align.c
+++ cfe/trunk/test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with 
single-byte alignment in older versions of GCC and Clang}}
+  char c:4;
+};
+
+#if defined(_WIN32)
+// On Windows clang uses MSVC compatible layout in this case.
+extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#else
+extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#endif
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -1073,17 +1073,14 @@
 TD->addAttr(::new (S.Context) PackedAttr(Attr.getRange(), S.Context,
 Attr.getAttributeSpellingListIndex()));
   else if (FieldDecl *FD = dyn_cast(D)) {
-// If the alignment is less than or equal to 8 bits, the packed attribute
-// has no effect.
+// Report warning about changed offset in the newer compiler versions.
 if (!FD->getType()->isDependentType() &&
-!FD->getType()->isIncompleteType() &&
+!FD->getType()->isIncompleteType() && FD->isBitField() &&
 S.Context.getTypeAlign(FD->getType()) <= 8)
-  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored_for_field_of_type)
-<< Attr.getName() << FD->getType();
-else
-  FD->addAttr(::new (S.Context)
-  PackedAttr(Attr.getRange(), S.Context,
- Attr.getAttributeSpellingListIndex()));
+  S.Diag(Attr.getLoc(), diag::warn_attribute_packed_for_bitfield);
+
+FD->addAttr(::new (S.Context) PackedAttr(
+Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
   } else
 S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
 }


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2784,9 +2784,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
-def warn_attribute_ignored_for_field_of_type : Warning<
-  "%0 attribute ignored for field of type %1">,
-  InGroup;
+def warn_attribute_packed_for_bitfield : Warning<
+  "'packed' attribute was ignored on bit-fields with single-byte alignment "
+  "in older versions of GCC and Clang">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<
   

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2788
@@ +2787,3 @@
+def warn_attribute_packed_for_bitfield : Warning<
+  "'packed' attribute was ignored on bit-fields with alignment 1 "
+  "in older versions of GCC and Clang">,

Would this read better as "on bit-fields with single-byte alignment" instead of 
"on bit-fields with alignment 1"?


Comment at: test/Sema/struct-packed-align.c:155
@@ +154,3 @@
+#if defined(_WIN32)
+// On Windows clang ignores uses MSVC compatible layout in this case.
+extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];

"ignores uses" -> "uses"


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Aaron Ballman via cfe-commits]

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

LGTM! Thank you!


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[John McCall via cfe-commits]

rjmccall added a comment.

LGTM, thanks!


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 41648.
DmitryPolukhin marked 2 inline comments as done.
DmitryPolukhin added a comment.

Fixed warning and comment.


http://reviews.llvm.org/D14872

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/struct-packed-align.c

Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with 
alignment 1 in older versions of GCC and Clang}}
+  char c:4;
+};
+
+#if defined(_WIN32)
+// On Windows clang uses MSVC compatible layout in this case.
+extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#else
+extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#endif
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1036,17 +1036,14 @@
 TD->addAttr(::new (S.Context) PackedAttr(Attr.getRange(), S.Context,
 Attr.getAttributeSpellingListIndex()));
   else if (FieldDecl *FD = dyn_cast(D)) {
-// If the alignment is less than or equal to 8 bits, the packed attribute
-// has no effect.
+// Report warning about changed offset in the newer compiler versions.
 if (!FD->getType()->isDependentType() &&
-!FD->getType()->isIncompleteType() &&
+!FD->getType()->isIncompleteType() && FD->isBitField() &&
 S.Context.getTypeAlign(FD->getType()) <= 8)
-  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored_for_field_of_type)
-<< Attr.getName() << FD->getType();
-else
-  FD->addAttr(::new (S.Context)
-  PackedAttr(Attr.getRange(), S.Context,
- Attr.getAttributeSpellingListIndex()));
+  S.Diag(Attr.getLoc(), diag::warn_attribute_packed_for_bitfield);
+
+FD->addAttr(::new (S.Context) PackedAttr(
+Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
   } else
 S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
 }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2784,9 +2784,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
-def warn_attribute_ignored_for_field_of_type : Warning<
-  "%0 attribute ignored for field of type %1">,
-  InGroup;
+def warn_attribute_packed_for_bitfield : Warning<
+  "'packed' attribute was ignored on bit-fields with single-byte alignment "
+  "in older versions of GCC and Clang">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<
   "%select{alignment|size}0 of field %1 (%2 bits) does not match the "
   "%select{alignment|size}0 of the first field in transparent union; "


Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{'packed' attribute was ignored on bit-fields 

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[John McCall via cfe-commits]

rjmccall added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2790
@@ -2790,1 +2789,3 @@
+  "version 4.4 - the newer offset is used here">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<

No, this diagnostic shouldn't make such an affirmative statement about the 
offset changing.  I would instead suggest:
  warning: 'packed' attribute was ignored on bit-fields with alignment 1 in 
older versions of GCC and Clang


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 41510.
DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added a comment.

Don't call getDeclName() that it is not required. PTAL


http://reviews.llvm.org/D14872

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/struct-packed-align.c

Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{the offset assigned to packed bit-field member 'b' 
changed with GCC version 4.4 - the newer offset is used here}}
+  char c:4;
+};
+
+#if defined(_WIN32)
+// On Windows clang ignores uses MSVC compatible layout in this case.
+extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#else
+extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#endif
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1036,17 +1036,15 @@
 TD->addAttr(::new (S.Context) PackedAttr(Attr.getRange(), S.Context,
 Attr.getAttributeSpellingListIndex()));
   else if (FieldDecl *FD = dyn_cast(D)) {
-// If the alignment is less than or equal to 8 bits, the packed attribute
-// has no effect.
+// Report warning about changed offset in the newer compiler versions.
 if (!FD->getType()->isDependentType() &&
-!FD->getType()->isIncompleteType() &&
+!FD->getType()->isIncompleteType() && FD->isBitField() &&
 S.Context.getTypeAlign(FD->getType()) <= 8)
-  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored_for_field_of_type)
-<< Attr.getName() << FD->getType();
-else
-  FD->addAttr(::new (S.Context)
-  PackedAttr(Attr.getRange(), S.Context,
- Attr.getAttributeSpellingListIndex()));
+  S.Diag(Attr.getLoc(),
+ diag::note_attribute_packed_for_bitfield_offset_changed) << FD;
+
+FD->addAttr(::new (S.Context) PackedAttr(
+Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
   } else
 S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
 }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2784,9 +2784,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
-def warn_attribute_ignored_for_field_of_type : Warning<
-  "%0 attribute ignored for field of type %1">,
-  InGroup;
+def note_attribute_packed_for_bitfield_offset_changed : Warning<
+  "the offset assigned to packed bit-field member %0 changed with GCC "
+  "version 4.4 - the newer offset is used here">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<
   "%select{alignment|size}0 of field %1 (%2 bits) does not match the "
   "%select{alignment|size}0 of the first field in transparent union; "


Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

This is another GCC ABI compatibility issue. If there is no more comments, 
could someone please approve this CL?


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: lib/Sema/SemaDeclAttr.cpp:1046
@@ +1045,3 @@
+ diag::note_attribute_packed_for_bitfield_offset_changed)
+<< FD->getDeclName();
+

No need to call getDeclName(), the diagnostic can accept a NamedDecl directly 
(which properly quotes the name in the diagnostic).


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 41222.
DmitryPolukhin marked 2 inline comments as done.
DmitryPolukhin added a comment.

Changed note text message + fixed outdated comment.


http://reviews.llvm.org/D14872

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/struct-packed-align.c

Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{the offset assigned to packed bit-field member 'b' 
changed with GCC version 4.4 - the newer offset is used here}}
+  char c:4;
+};
+
+#if defined(_WIN32)
+// On Windows clang ignores uses MSVC compatible layout in this case.
+extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#else
+extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#endif
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1036,17 +1036,17 @@
 TD->addAttr(::new (S.Context) PackedAttr(Attr.getRange(), S.Context,
 Attr.getAttributeSpellingListIndex()));
   else if (FieldDecl *FD = dyn_cast(D)) {
-// If the alignment is less than or equal to 8 bits, the packed attribute
-// has no effect.
+// Report warning about changed offset in the newer compiler versions.
 if (!FD->getType()->isDependentType() &&
 !FD->getType()->isIncompleteType() &&
+FD->isBitField() &&
 S.Context.getTypeAlign(FD->getType()) <= 8)
-  S.Diag(Attr.getLoc(), diag::warn_attribute_ignored_for_field_of_type)
-<< Attr.getName() << FD->getType();
-else
-  FD->addAttr(::new (S.Context)
-  PackedAttr(Attr.getRange(), S.Context,
- Attr.getAttributeSpellingListIndex()));
+  S.Diag(Attr.getLoc(),
+ diag::note_attribute_packed_for_bitfield_offset_changed)
+<< FD->getDeclName();
+
+FD->addAttr(::new (S.Context) PackedAttr(
+Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
   } else
 S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
 }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2780,9 +2780,10 @@
   "cast to %1 from smaller integer type %0">,
   InGroup;
 
-def warn_attribute_ignored_for_field_of_type : Warning<
-  "%0 attribute ignored for field of type %1">,
-  InGroup;
+def note_attribute_packed_for_bitfield_offset_changed : Warning<
+  "the offset assigned to packed bit-field member %0 changed with GCC "
+  "version 4.4 - the newer offset is used here">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<
   "%select{alignment|size}0 of field %1 (%2 bits) does not match the "
   "%select{alignment|size}0 of the first field in transparent union; "


Index: test/Sema/struct-packed-align.c
===
--- test/Sema/struct-packed-align.c
+++ test/Sema/struct-packed-align.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
 
 // Packed structs.
 struct s {
@@ -138,3 +138,24 @@
 extern int n1[sizeof(struct nS) == 9 ? 1 : -1];
 extern int n2[__alignof(struct nS) == 1 ? 1 : -1];
 #endif
+
+// Packed attribute shouldn't be ignored for bit-field of char types.
+// Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
+// the packed attribute on bit-fields of type char. This has been fixed in
+// GCC 4.4 but the change can lead to differences in the structure layout.
+// See the documentation of -Wpacked-bitfield-compat for more information.
+struct packed_chars {
+  char a:4;
+  char 

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Richard Smith via cfe-commits]

On Nov 25, 2015 1:53 PM, "hfin...@anl.gov via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:
>
> hfinkel added inline comments.
>
> 
> Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2783
> @@ -2783,1 +2782,3 @@
> +  "the newer semantic is provided here">,
> +  InGroup>;
>  def warn_transparent_union_attribute_field_size_align : Warning<
> 
> Calling this "a semantic" reads oddly to me. This sounds better to me:
>
>   def note_attribute_packed_for_bitfield_offset_changed : Warning<
> "the offset assigned to packed bit-field member %0 has changed with
GCC version 4.4 - "

Please also remove the "has" here.

> "the newer offset is used here">,
> InGroup>;
>
> 
> Comment at: lib/Sema/SemaDeclAttr.cpp:1040
> @@ -1039,3 +1039,3 @@
>  // If the alignment is less than or equal to 8 bits, the packed
attribute
>  // has no effect.
>  if (!FD->getType()->isDependentType() &&
> 
> This comment is now out of date?
>
>
> http://reviews.llvm.org/D14872
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[hfin...@anl.gov via cfe-commits]

hfinkel added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2783
@@ -2783,1 +2782,3 @@
+  "the newer semantic is provided here">,
+  InGroup>;
 def warn_transparent_union_attribute_field_size_align : Warning<

Calling this "a semantic" reads oddly to me. This sounds better to me:

  def note_attribute_packed_for_bitfield_offset_changed : Warning<
"the offset assigned to packed bit-field member %0 has changed with GCC 
version 4.4 - "
"the newer offset is used here">,
InGroup>;


Comment at: lib/Sema/SemaDeclAttr.cpp:1040
@@ -1039,3 +1039,3 @@
 // If the alignment is less than or equal to 8 bits, the packed attribute
 // has no effect.
 if (!FD->getType()->isDependentType() &&

This comment is now out of date?


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[hfin...@anl.gov via cfe-commits]

hfinkel added a subscriber: hfinkel.
hfinkel added a comment.

Please upload this patch with full context, see: 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

> Should we add warning about changes in layout for packed bit-fileds of char 
> type? GCC has it "note: offset of packed bit-field ‘b’ has changed in GCC 
> 4.4" will add if needed.


Warning about this is a good idea. We did something similar when we 
(re-)introduced sync_fetch_and_nand, see:

  def warn_sync_fetch_and_nand_semantics_change : Warning<
"the semantics of this intrinsic changed with GCC "
"version 4.4 - the newer semantics are provided here">,
InGroup>;


http://reviews.llvm.org/D14872



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

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

It seems that check for type alignment <= 8 was there practically forever 
http://llvm.org/viewvc/llvm-project/cfe/trunk/Sema/SemaDecl.cpp?r1=47197=47196=47197
 and there is no good explanation why it was implemented. Subsequent changes 
only add more condition to exclude cases when it shouldn't be reported.


http://reviews.llvm.org/D14872



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