[clang] [clang] Respect field alignment in layout compatibility of structs (PR #84313)
Vlad Serebrennikov via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 7 13:19:32 PST 2024
https://github.com/Endilll updated https://github.com/llvm/llvm-project/pull/84313
>From 491fc16c777aff8b22893da1cdeb8d137cf28871 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Thu, 7 Mar 2024 15:16:35 +0300
Subject: [PATCH 1/4] [clang] Respect field alignment when evaluating layout
compatiblity of structs
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).
---
clang/docs/ReleaseNotes.rst | 4 ++++
clang/lib/Sema/SemaChecking.cpp | 9 +++++++--
clang/test/CXX/drs/dr25xx.cpp | 26 ++++++++++++++++++++++++++
clang/test/SemaCXX/type-traits.cpp | 13 ++++++++++++-
clang/www/cxx_dr_status.html | 2 +-
5 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1b901a27fd19d1..62cc6aae4ee58b 100644
--- 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
index 3597f93a017136..5f607608cf7a53 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19185,7 +19185,8 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
/// Check if two fields are layout-compatible.
static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
- FieldDecl *Field2) {
+ FieldDecl *Field2,
+ bool IgnoreAlignment = false) {
if (!isLayoutCompatible(C, Field1->getType(), Field2->getType()))
return false;
@@ -19204,6 +19205,10 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
if (Field1->hasAttr<clang::NoUniqueAddressAttr>() ||
Field2->hasAttr<clang::NoUniqueAddressAttr>())
return false;
+
+ if (!IgnoreAlignment &&
+ Field1->getMaxAlignment() != Field2->getMaxAlignment())
+ return false;
return true;
}
@@ -19265,7 +19270,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, /*IgnoreAlignment=*/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
index 9fc7cf59485caa..46532486e50e53 100644
--- 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
index 23c339ebdf0826..831de2589dcb9e 100644
--- 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
index 774c71bc1cb6b7..494c0e1e9007bd 100755
--- 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>
>From c0110bdaa3d6400c00322c79e59b343888fea8a5 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Thu, 7 Mar 2024 21:46:08 +0300
Subject: [PATCH 2/4] Rename `IgnoreAlignment` parameter to `IsUnionMember`
---
clang/lib/Sema/SemaChecking.cpp | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5f607608cf7a53..6feb7830796a50 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19185,8 +19185,7 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
/// Check if two fields are layout-compatible.
static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
- FieldDecl *Field2,
- bool IgnoreAlignment = false) {
+ FieldDecl *Field2, bool IsUnionMember = false) {
if (!isLayoutCompatible(C, Field1->getType(), Field2->getType()))
return false;
@@ -19206,8 +19205,7 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
Field2->hasAttr<clang::NoUniqueAddressAttr>())
return false;
- if (!IgnoreAlignment &&
- Field1->getMaxAlignment() != Field2->getMaxAlignment())
+ if (!IsUnionMember && Field1->getMaxAlignment() != Field2->getMaxAlignment())
return false;
return true;
}
@@ -19270,7 +19268,7 @@ static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
E = UnmatchedFields.end();
for ( ; I != E; ++I) {
- if (isLayoutCompatible(C, Field1, *I, /*IgnoreAlignment=*/true)) {
+ if (isLayoutCompatible(C, Field1, *I, /*IsUnionMember=*/true)) {
bool Result = UnmatchedFields.erase(*I);
(void) Result;
assert(Result);
>From 5454f15d2270da5fa38b427eaa4842f6e5d09d94 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Thu, 7 Mar 2024 23:42:10 +0300
Subject: [PATCH 3/4] Add a comment for new boolean parameter and an assert
---
clang/lib/Sema/SemaChecking.cpp | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 6feb7830796a50..edf080bd686db0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19184,8 +19184,19 @@ 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, bool IsUnionMember = false) {
+ 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.");
+
if (!isLayoutCompatible(C, Field1->getType(), Field2->getType()))
return false;
@@ -19205,8 +19216,10 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
Field2->hasAttr<clang::NoUniqueAddressAttr>())
return false;
- if (!IsUnionMember && Field1->getMaxAlignment() != Field2->getMaxAlignment())
+ if (!AreUnionMembers &&
+ Field1->getMaxAlignment() != Field2->getMaxAlignment())
return false;
+
return true;
}
>From cc2039bbca06727b044331d845b325e5ef1f23b4 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Fri, 8 Mar 2024 00:19:14 +0300
Subject: [PATCH 4/4] Add an additional assert for the value of
`AreUnionMembers`
---
clang/lib/Sema/SemaChecking.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index edf080bd686db0..b34b8df0020137 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19196,6 +19196,9 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
(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;
More information about the cfe-commits
mailing list