Address feedback

Author: ojhunt

clang/include/clang/Basic/DiagnosticGroups.td

@@ -635,9 +635,10 @@ def Padded : DiagGroup<"padded", [PaddedBitField]>;
 def UnalignedAccess : DiagGroup<"unaligned-access">;
 def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-compatibility"> {
   code Documentation = [{
-    Under the MSVC ABI adjacent bit-fields are not packed if the underlying type
-    has a different storage size. This warning indicates that a pair of adjacent
-    bit-fields may not pack in the same way due to this behavioural difference.
+    Under the MS Windows platform ABI, adjacent bit-fields are not packed if the
+    underlying type has a different storage size. This warning indicates that a
+    pair of adjacent bit-fields may not pack in the same way due to this behavioural
+    difference.
 
     This can occur when mixing different types explicitly:
 
@@ -659,13 +660,13 @@ def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-compatibility"> {
         Enum2 field2 : 1;
       };
 
-    In each of these cases under the MSVC ABI the second bit-field will not be
-    packed with the preceding bit-field, and instead will be aligned as if the
-    fields were each separately defined integer fields of their respective storage
-    size. For binary compatibility this is obviously and observably incompatible,
-    however where bit-fields are being used solely for memory use reduction this
-    incomplete packing may silently increase the size of objects vs what is
-    expected.
+    In each of these cases under the MS Windows platform ABI the second bit-field
+    will not be packed with the preceding bit-field, and instead will be aligned
+    as if the fields were each separately defined integer fields of their respective
+    storage size. For binary compatibility this is obviously and observably
+    incompatible, however where bit-fields are being used solely for memory use
+    reduction this incomplete packing may silently increase the size of objects vs
+    what is expected.
 
     This issue can be addressed by ensuring the storage type of each bit-field is
     the same, either by explicitly using the same integer type, or in the case of

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6425,7 +6425,7 @@ def note_change_bitfield_sign : Note<
 def warn_ms_bitfield_mismatched_storage_packing : Warning<
   "bit-field %0 of type %1 has a different storage size than the "
   "preceding bit-field (%2 vs %3 bytes) and will not be packed under "
-  "the MSVC ABI">,
+  "the MS Windows platform ABI">,
   InGroup<MSBitfieldCompatibility>, DefaultIgnore;
 def note_ms_bitfield_mismatched_storage_size_previous : Note<
   "preceding bit-field %0 declared here with type %1">;

clang/test/SemaCXX/ms_struct-bitfield-padding.cpp

@@ -55,7 +55,7 @@ static_assert(sizeof(F) == 4);
 struct G {
   EnumU32_1 a : 15;
   EnumU64 b : 15;
-  // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
 };
 
@@ -76,7 +76,7 @@ struct I {
   EnumU8 a : 3;
   EnumI8 b : 5;
   EnumU32_1 c : 10;
-  // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'b' declared here with type 'EnumI8'}}
 };
 #ifdef MS_BITFIELDS
@@ -116,10 +116,10 @@ static_assert(sizeof(M) == 4);
 struct N {
   EnumU32_1 a : 10;
   EnumU64 b : 10;
-  // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
   EnumU32_1 c : 10;
-  // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-5 {{preceding bit-field 'b' declared here with type 'EnumU64'}}
 };
 
@@ -132,7 +132,7 @@ static_assert(sizeof(N) == 8);
 struct O {
   EnumU16 a : 10;
   EnumU32_1 b : 10;
-  // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU16'}}
 };
 #ifdef MS_BITFIELDS
@@ -144,7 +144,7 @@ static_assert(sizeof(O) == 4);
 struct P {
   EnumU32_1 a : 10;
   EnumU16 b : 10;
-  // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
 };
 #ifdef MS_BITFIELDS
@@ -156,7 +156,7 @@ static_assert(sizeof(P) == 4);
 struct Q {
   EnumU8 a : 6;
   EnumU16 b : 6;
-  // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU8'}}
 };
 #ifdef MS_BITFIELDS
@@ -169,7 +169,7 @@ struct R {
   EnumU16 a : 9;
   EnumU16 b : 9;
   EnumU8 c : 6;
-  // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed under the MSVC ABI}}
+  // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed under the MS Windows platform ABI}}
   // expected-note@-3 {{preceding bit-field 'b' declared here with type 'EnumU16'}}
 };
 
@@ -178,3 +178,19 @@ static_assert(sizeof(R) == 6);
 #else
 static_assert(sizeof(R) == 4);
 #endif
+
+struct S {
+  char a : 4;
+  char b : 4;
+  char c : 4;
+  char d : 4;
+  short x : 7;
+  // expected-warning@-1 {{bit-field 'x' of type 'short' has a different storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the MS Windows platform ABI}}
+  // expected-note@-3 {{preceding bit-field 'd' declared here with type 'char'}}
+  // This is a false positive. Reporting this correctly requires duplicating the record layout process
+  // in target and MS layout modes, and it's also unclear if that's the correct choice for users of
+  // this diagnostic.
+  short y : 9;
+};
+
+static_assert(sizeof(S) == 4);