Address feedback

Author: ojhunt

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6423,8 +6423,9 @@ def warn_signed_bitfield_enum_conversion : Warning<
 def note_change_bitfield_sign : Note<
   "consider making the bit-field type %select{unsigned|signed}0">;
 def warn_ms_bitfield_mismatched_storage_packing : Warning<
-  "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than the "
-  "preceding bit-field and may not be packed under MSVC ABI">,
+  "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">,
   InGroup<MSBitfieldCompatibility>, DefaultIgnore;
 def note_ms_bitfield_mismatched_storage_size_previous : Note<
   "preceding bit-field %0 declared here with type %1">;

clang/lib/Sema/SemaDecl.cpp

@@ -19016,7 +19016,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
 
   // Verify that all the fields are okay.
   SmallVector<FieldDecl*, 32> RecFields;
-  std::optional<const FieldDecl *> PreviousField;
+  const FieldDecl *PreviousField = nullptr;
   for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
        i != end; PreviousField = cast<FieldDecl>(*i), ++i) {
     FieldDecl *FD = cast<FieldDecl>(*i);
@@ -19229,26 +19229,22 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
     if (Record && FD->getType().isVolatileQualified())
       Record->setHasVolatileMember(true);
     auto IsNonDependentBitField = [](const FieldDecl *FD) {
-      if (!FD->isBitField())
-        return false;
-      if (FD->getType()->isDependentType())
-        return false;
-      return true;
+      return FD->isBitField() && !FD->getType()->isDependentType();
     };
 
     if (Record && PreviousField && IsNonDependentBitField(FD) &&
-        IsNonDependentBitField(*PreviousField)) {
-      unsigned FDStorageSize =
-          Context.getTypeSizeInChars(FD->getType()).getQuantity();
-      unsigned PreviousFieldStorageSize =
-          Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+        IsNonDependentBitField(PreviousField)) {
+      CharUnits FDStorageSize = Context.getTypeSizeInChars(FD->getType());
+      CharUnits PreviousFieldStorageSize =
+          Context.getTypeSizeInChars(PreviousField->getType());
       if (FDStorageSize != PreviousFieldStorageSize) {
         Diag(FD->getLocation(),
              diag::warn_ms_bitfield_mismatched_storage_packing)
-            << FD << FD->getType() << FDStorageSize << PreviousFieldStorageSize;
-        Diag((*PreviousField)->getLocation(),
+            << FD << FD->getType() << FDStorageSize.getQuantity()
+            << PreviousFieldStorageSize.getQuantity();
+        Diag(PreviousField->getLocation(),
              diag::note_ms_bitfield_mismatched_storage_size_previous)
-            << *PreviousField << (*PreviousField)->getType();
+            << PreviousField << PreviousField->getType();
       }
     }
     // Keep track of the number of named members.

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 (8 vs 4 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (4 vs 1 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (8 vs 4 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (4 vs 8 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (4 vs 2 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (2 vs 4 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (2 vs 1 bytes) than the preceding bit-field and may not be packed under 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 MSVC 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 (1 vs 2 bytes) than the preceding bit-field and may not be packed under 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 MSVC ABI}}
   // expected-note@-3 {{preceding bit-field 'b' declared here with type 'EnumU16'}}
 };