Skip to content

[Clang] Fix definition of layout-compatible to ignore empty classes #92103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ Bug Fixes in This Version
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- ``__is_layout_compatible`` no longer requires the empty bases to be the same in two
standard-layout classes. It now only compares non-static data members.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I was under the impression that this feature had not been previously released; is that incorrect?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct, but the feature made into the 19 branch, whereas this fix did not (I hope we can backport it), hence a release note.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the plan to remove the release note from LLVM 20 if we're able to backport the fix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but this indeed is a possible course of action.
If you have preferences how this should be handled, let us know.
Personally I'm more focused on getting this backported.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should backport these changes, drop the Clang 20 release note, and no need for a Clang 19.x release note to be added.


Bug Fixes to Attribute Support
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,13 @@ class CXXRecordDecl : public RecordDecl {
return D.HasPublicFields || D.HasProtectedFields || D.HasPrivateFields;
}

/// If this is a standard-layout class or union, any and all data members will
/// be declared in the same type.
///
/// This retrieves the type where any fields are declared,
/// or the current class if there is no class with fields.
const CXXRecordDecl *getStandardLayoutBaseWithFields() const;

/// Whether this class is polymorphic (C++ [class.virtual]),
/// which means that the class contains or inherits a virtual function.
bool isPolymorphic() const { return data().Polymorphic; }
Expand Down
36 changes: 36 additions & 0 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
data().StructuralIfLiteral = false;
}

const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
assert(
isStandardLayout() &&
"getStandardLayoutBaseWithFields called on a non-standard-layout type");
#ifdef EXPENSIVE_CHECKS
{
unsigned NumberOfBasesWithFields = 0;
if (!field_empty())
++NumberOfBasesWithFields;
llvm::SmallPtrSet<const CXXRecordDecl *, 8> UniqueBases;
forallBases([&](const CXXRecordDecl *Base) -> bool {
if (!Base->field_empty())
++NumberOfBasesWithFields;
assert(
UniqueBases.insert(Base->getCanonicalDecl()).second &&
"Standard layout struct has multiple base classes of the same type");
return true;
});
assert(NumberOfBasesWithFields <= 1 &&
"Standard layout struct has fields declared in more than one class");
}
#endif
if (!field_empty())
return this;
const CXXRecordDecl *Result = this;
forallBases([&](const CXXRecordDecl *Base) -> bool {
if (!Base->field_empty()) {
// This is the base where the fields are declared; return early
Result = Base;
return false;
}
return true;
});
return Result;
}

bool CXXRecordDecl::hasConstexprDestructor() const {
auto *Dtor = getDestructor();
return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr();
Expand Down
74 changes: 24 additions & 50 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13706,10 +13706,11 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,

//===--- Layout compatibility ----------------------------------------------//

static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
static bool isLayoutCompatible(const ASTContext &C, QualType T1, QualType T2);

/// Check if two enumeration types are layout-compatible.
static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
static bool isLayoutCompatible(const ASTContext &C, const EnumDecl *ED1,
const EnumDecl *ED2) {
// C++11 [dcl.enum] p8:
// Two enumeration types are layout-compatible if they have the same
// underlying type.
Expand All @@ -13720,8 +13721,8 @@ 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,
static bool isLayoutCompatible(const ASTContext &C, const FieldDecl *Field1,
const FieldDecl *Field2,
bool AreUnionMembers = false) {
[[maybe_unused]] const Type *Field1Parent =
Field1->getParent()->getTypeForDecl();
Expand Down Expand Up @@ -13764,60 +13765,33 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,

/// Check if two standard-layout structs are layout-compatible.
/// (C++11 [class.mem] p17)
static bool isLayoutCompatibleStruct(ASTContext &C, RecordDecl *RD1,
RecordDecl *RD2) {
// If both records are C++ classes, check that base classes match.
if (const CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(RD1)) {
// If one of records is a CXXRecordDecl we are in C++ mode,
// thus the other one is a CXXRecordDecl, too.
const CXXRecordDecl *D2CXX = cast<CXXRecordDecl>(RD2);
// Check number of base classes.
if (D1CXX->getNumBases() != D2CXX->getNumBases())
return false;
static bool isLayoutCompatibleStruct(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
// Get to the class where the fields are declared
if (const CXXRecordDecl *D1CXX = dyn_cast<CXXRecordDecl>(RD1))
RD1 = D1CXX->getStandardLayoutBaseWithFields();

// Check the base classes.
for (CXXRecordDecl::base_class_const_iterator
Base1 = D1CXX->bases_begin(),
BaseEnd1 = D1CXX->bases_end(),
Base2 = D2CXX->bases_begin();
Base1 != BaseEnd1;
++Base1, ++Base2) {
if (!isLayoutCompatible(C, Base1->getType(), Base2->getType()))
return false;
}
} else if (const CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(RD2)) {
// If only RD2 is a C++ class, it should have zero base classes.
if (D2CXX->getNumBases() > 0)
return false;
}
if (const CXXRecordDecl *D2CXX = dyn_cast<CXXRecordDecl>(RD2))
RD2 = D2CXX->getStandardLayoutBaseWithFields();

// Check the fields.
RecordDecl::field_iterator Field2 = RD2->field_begin(),
Field2End = RD2->field_end(),
Field1 = RD1->field_begin(),
Field1End = RD1->field_end();
for ( ; Field1 != Field1End && Field2 != Field2End; ++Field1, ++Field2) {
if (!isLayoutCompatible(C, *Field1, *Field2))
return false;
}
if (Field1 != Field1End || Field2 != Field2End)
return false;

return true;
return llvm::equal(RD1->fields(), RD2->fields(),
[&C](const FieldDecl *F1, const FieldDecl *F2) -> bool {
return isLayoutCompatible(C, F1, F2);
});
}

/// Check if two standard-layout unions are layout-compatible.
/// (C++11 [class.mem] p18)
static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
RecordDecl *RD2) {
llvm::SmallPtrSet<FieldDecl *, 8> UnmatchedFields;
static bool isLayoutCompatibleUnion(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
llvm::SmallPtrSet<const FieldDecl *, 8> UnmatchedFields;
for (auto *Field2 : RD2->fields())
UnmatchedFields.insert(Field2);

for (auto *Field1 : RD1->fields()) {
llvm::SmallPtrSet<FieldDecl *, 8>::iterator
I = UnmatchedFields.begin(),
E = UnmatchedFields.end();
auto I = UnmatchedFields.begin();
auto E = UnmatchedFields.end();

for ( ; I != E; ++I) {
if (isLayoutCompatible(C, Field1, *I, /*IsUnionMember=*/true)) {
Expand All @@ -13834,8 +13808,8 @@ static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
return UnmatchedFields.empty();
}

static bool isLayoutCompatible(ASTContext &C, RecordDecl *RD1,
RecordDecl *RD2) {
static bool isLayoutCompatible(const ASTContext &C, const RecordDecl *RD1,
const RecordDecl *RD2) {
if (RD1->isUnion() != RD2->isUnion())
return false;

Expand All @@ -13846,7 +13820,7 @@ static bool isLayoutCompatible(ASTContext &C, RecordDecl *RD1,
}

/// Check if two types are layout-compatible in C++11 sense.
static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2) {
static bool isLayoutCompatible(const ASTContext &C, QualType T1, QualType T2) {
if (T1.isNull() || T2.isNull())
return false;

Expand Down
11 changes: 11 additions & 0 deletions clang/test/SemaCXX/type-traits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,11 @@ struct CStructWithFMA2 {
int f[];
};

template<int N>
struct UniqueEmpty {};
template<typename... Bases>
struct D : Bases... {};

void is_layout_compatible(int n)
{
static_assert(__is_layout_compatible(void, void));
Expand Down Expand Up @@ -1841,6 +1846,12 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumClassLayout, int));
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
static_assert(__is_layout_compatible(CStruct, D<CStruct>));
static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, CStruct>));
static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, D<UniqueEmpty<1>, CStruct>, D<UniqueEmpty<2>>>));
static_assert(__is_layout_compatible(CStruct, D<CStructWithQualifiers>));
static_assert(__is_layout_compatible(CStruct, D<UniqueEmpty<0>, CStructWithQualifiers>));
static_assert(__is_layout_compatible(CStructWithQualifiers, D<UniqueEmpty<0>, D<UniqueEmpty<1>, CStruct>, D<UniqueEmpty<2>>>));
}

namespace IPIBO {
Expand Down
6 changes: 6 additions & 0 deletions llvm/include/llvm/ADT/STLExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -2027,6 +2027,12 @@ template <typename L, typename R> bool equal(L &&LRange, R &&RRange) {
adl_end(RRange));
}

template <typename L, typename R, typename BinaryPredicate>
bool equal(L &&LRange, R &&RRange, BinaryPredicate P) {
return std::equal(adl_begin(LRange), adl_end(LRange), adl_begin(RRange),
adl_end(RRange), P);
}

/// Returns true if all elements in Range are equal or when the Range is empty.
template <typename R> bool all_equal(R &&Range) {
auto Begin = adl_begin(Range);
Expand Down
Loading