[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)

[Oliver Hunt via cfe-commits]

================
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-compatibility -verify -triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields -verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
+enum class EnumI8 : char { A, B };
+enum class EnumU16 : unsigned short { A, B };
+enum class EnumI16 : short { A, B };
+
+struct A {
+  unsigned int a : 15;
+  unsigned int b : 15;
+};
+static_assert(sizeof(A) == 4);
+
+struct B {
+  unsigned int a : 15;
+           int b : 15;
+};
+static_assert(sizeof(B) == 4);
+
+struct C {
+  unsigned int a : 15;
+           int b : 15;
+};
+static_assert(sizeof(C) == 4);
+
+struct D {
+  Enum1 a : 15;
+  Enum1 b : 15;
+};
+static_assert(sizeof(D) == 4);
+
+struct E {
+  Enum1 a : 15;
+  Enum2 b : 15;
+};
+static_assert(sizeof(E) == 4);
+
+struct F {
+  EnumU32_1 a : 15;
+  EnumU32_2 b : 15;
+};
+static_assert(sizeof(F) == 4);
+
+struct G {
+  EnumU32_1 a : 15;
+  EnumU64 b : 15;
+  // expected-warning at -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 at -3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
+};
+
+#ifdef MS_BITFIELDS
+  static_assert(sizeof(G) == 16);
+#else
+  static_assert(sizeof(G) == 8);
+#endif
+
+struct H {
+  EnumU32_1 a : 10;
+  EnumI32 b : 10;
+  EnumU32_1 c : 10;
+};
+static_assert(sizeof(H) == 4);
+
+struct I {
+  EnumU8 a : 3;
+  EnumI8 b : 5;
+  EnumU32_1 c : 10;
+  // expected-warning at -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 at -3 {{preceding bit-field 'b' declared here with type 'EnumI8'}}
+};
+#ifdef MS_BITFIELDS
+static_assert(sizeof(I) == 8);
+#else
+static_assert(sizeof(I) == 4);
+#endif
+
+struct J {
+  EnumU8 : 0;
+  EnumU8 b : 4;
+};
+static_assert(sizeof(J) == 1);
+
+struct K {
+  EnumU8 a : 4;
+  EnumU8 : 0;
+};
+static_assert(sizeof(K) == 1);
+
+struct L {
+  EnumU32_1 a : 10;
+  EnumU32_2 b : 10;
+  EnumU32_1 c : 10;
+};
+
+static_assert(sizeof(L) == 4);
+
+struct M {
+  EnumU32_1 a : 10;
+  EnumI32 b : 10;
+  EnumU32_1 c : 10;
+};
+
+static_assert(sizeof(M) == 4);
+
----------------
ojhunt wrote:

@rnk fun story: my first pass at this was in record layout and it involved layout out the record twice and was awful :D - it also reminded me that there are a bunch of warnings that we don't issue until the point of use rather than declaration which is irksome.

That said, while my original implementation was in record layout and was intended to be 100% accurate,  I realized that the actual issue is different type storage sizes resulting in radically different behavior with *seemingly unrelated* changes later on. So in cases where it matters we just want all bit-fields to be the same storage size, and the warning currently just drives "all bit-fields shall be the same storage size" which is the safe option. Pushing it to actual accurate warnings seems like it _might_ be less than ideal, as you get cases like this trivial change to the above false positive

```cpp
struct Foo {
  char a : 4;
  char b : 4;
  char c : 4;
  char d : 4;
  char e : 1; // +1 bit field
  short x : 6; // -1 bit width
  short y : 9;
};
```

which would now issue a warning saying that it is no longer packed on msvc, so the fix will be to go through and restructure/re-type (type system type, not keyboard) the entire structure to ensure everything has the same storage type, when it may have been easier to have it "correct" (from msvc point of view) from the start.

e.g the early false positive forces the code to be written in a way that avoids a real warning down the line that requires more changes. That's also why the warning text originally said "may" :D 

https://github.com/llvm/llvm-project/pull/117428