[clang] Respect field alignment in layout compatibility of structs (#…

…84313) This patch implements [CWG2586](https://cplusplus.github.io/CWG/issues/2583.html) "Common initial sequence should consider over-alignment". Note that alignment of union members doesn't have to match, as layout compatibility of unions is not defined in terms of common initial sequence (http://eel.is/c++draft/class.mem.general#25).
llvm · Mar 8, 2024 · b6a3400 · b6a3400
1 parent f6b825f
commit b6a3400
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 4 deletions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -115,6 +115,10 @@ Resolutions to C++ Defect Reports
   of two types.
   (`CWG1719: Layout compatibility and cv-qualification revisited <https://cplusplus.github.io/CWG/issues/1719.html>`_).
 
+- Alignment of members is now respected when evaluating layout compatibility
+  of structs.
+  (`CWG2583: Common initial sequence should consider over-alignment <https://cplusplus.github.io/CWG/issues/2583.html>`_).
+
 - ``[[no_unique_address]]`` is now respected when evaluating layout
   compatibility of two types.
   (`CWG2759: [[no_unique_address] and common initial sequence  <https://cplusplus.github.io/CWG/issues/2759.html>`_).

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -19184,8 +19184,22 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
 }
 
 /// Check if two fields are layout-compatible.
+/// Can be used on union members, which are exempt from alignment requirement
+/// of common initial sequence.
 static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
-                               FieldDecl *Field2) {
+                               FieldDecl *Field2,
+                               bool AreUnionMembers = false) {
+  const Type *Field1Parent = Field1->getParent()->getTypeForDecl();
+  const Type *Field2Parent = Field2->getParent()->getTypeForDecl();
+  assert(((Field1Parent->isStructureOrClassType() &&
+           Field2Parent->isStructureOrClassType()) ||
+          (Field1Parent->isUnionType() && Field2Parent->isUnionType())) &&
+         "Can't evaluate layout compatibility between a struct field and a "
+         "union field.");
+  assert(((!AreUnionMembers && Field1Parent->isStructureOrClassType()) ||
+          (AreUnionMembers && Field1Parent->isUnionType())) &&
+         "AreUnionMembers should be 'true' for union fields (only).");
+
   if (!isLayoutCompatible(C, Field1->getType(), Field2->getType()))
     return false;
 
@@ -19204,6 +19218,11 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
   if (Field1->hasAttr<clang::NoUniqueAddressAttr>() ||
       Field2->hasAttr<clang::NoUniqueAddressAttr>())
     return false;
+
+  if (!AreUnionMembers &&
+      Field1->getMaxAlignment() != Field2->getMaxAlignment())
+    return false;
+
   return true;
 }
 
@@ -19265,7 +19284,7 @@ static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
         E = UnmatchedFields.end();
 
     for ( ; I != E; ++I) {
-      if (isLayoutCompatible(C, Field1, *I)) {
+      if (isLayoutCompatible(C, Field1, *I, /*IsUnionMember=*/true)) {
         bool Result = UnmatchedFields.erase(*I);
         (void) Result;
         assert(Result);

diff --git a/clang/test/CXX/drs/dr25xx.cpp b/clang/test/CXX/drs/dr25xx.cpp
@@ -211,6 +211,32 @@ namespace dr2565 { // dr2565: 16 open 2023-06-07
 #endif
 }
 
+namespace dr2583 { // dr2583: 19
+#if __cplusplus >= 201103L
+struct A {
+  int i;
+  char c;
+};
+
+struct B {
+  int i;
+  alignas(8) char c;
+};
+
+union U {
+  A a;
+  B b;
+};
+
+union V {
+  A a;
+  alignas(64) B b;
+};
+
+static_assert(!__is_layout_compatible(A, B), "");
+static_assert(__is_layout_compatible(U, V), "");
+#endif
+} // namespace dr2583
 
 namespace dr2598 { // dr2598: 18
 #if __cplusplus >= 201103L

diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
@@ -1681,6 +1681,16 @@ union UnionLayout3 {
   [[no_unique_address]] CEmptyStruct d;
 };
 
+union UnionNoOveralignedMembers {
+  int a;
+  double b;
+};
+
+union UnionWithOveralignedMembers {
+  int a;
+  alignas(16) double b;
+};
+
 struct StructWithAnonUnion {
   union {
     int a;
@@ -1771,7 +1781,8 @@ void is_layout_compatible(int n)
   static_assert(__is_layout_compatible(CStruct, CStructNoUniqueAddress) != bool(__has_cpp_attribute(no_unique_address)), "");
   static_assert(__is_layout_compatible(CStructNoUniqueAddress, CStructNoUniqueAddress2) != bool(__has_cpp_attribute(no_unique_address)), "");
   static_assert(__is_layout_compatible(CStruct, CStructAlignment), "");
-  static_assert(__is_layout_compatible(CStruct, CStructAlignedMembers), ""); // FIXME: alignment of members impact common initial sequence
+  static_assert(!__is_layout_compatible(CStruct, CStructAlignedMembers), "");
+  static_assert(__is_layout_compatible(UnionNoOveralignedMembers, UnionWithOveralignedMembers), "");
   static_assert(__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds), "");
   static_assert(__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds2), "");
   static_assert(!__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds3), "");

diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
@@ -15306,7 +15306,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2583.html">2583</a></td>
     <td>C++23</td>
     <td>Common initial sequence should consider over-alignment</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 19</td>
   </tr>
   <tr class="open" id="2584">
     <td><a href="https://cplusplus.github.io/CWG/issues/2584.html">2584</a></td>