Skip to content
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

define_aggregate: provide empty user-defined constructor or destructor for unions with non-trivial members #119

Open
wants to merge 10 commits into
base: p2996
Choose a base branch
from
41 changes: 41 additions & 0 deletions clang/lib/AST/ExprConstantMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4532,6 +4532,47 @@ bool define_aggregate(APValue &Result, ASTContext &C, MetaActions &Meta,
if (!Definition)
return true;

if (Definition->isUnion()) {
Copy link
Author

Choose a reason for hiding this comment

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

decided to put this logic here instead of SemaReflect because this code is specific to function and doesn't require access to Sema

// union specific behaviour of "define_aggregate"

for (CXXConstructorDecl *CD : Definition->ctors()) {
if (CD->isDefaultConstructor() && CD->isDeleted()) {
CXXConstructorDecl *CanonicalCD = CD->getCanonicalDecl();

CanonicalCD->setImplicit(false);
CanonicalCD->setDeletedAsWritten(false);
CanonicalCD->setDefaulted(false);
CanonicalCD->setAccess(AS_public);

auto *EmptyBody =
CompoundStmt::CreateEmpty(CanonicalCD->getASTContext(), 0, false);
CanonicalCD->setBody(EmptyBody);

assert(CD->isUserProvided());
assert(CD->isDefaultConstructor());
assert(!CD->isDeleted());
}
}

if (CXXDestructorDecl *DD = Definition->getDestructor()) {
if (DD->isDeleted()) {
CXXDestructorDecl *CanonicalDD = DD->getCanonicalDecl();

CanonicalDD->setImplicit(false);
CanonicalDD->setDeletedAsWritten(false);
CanonicalDD->setDefaulted(false);
CanonicalDD->setAccess(AS_public);

auto *EmptyBody =
CompoundStmt::CreateEmpty(CanonicalDD->getASTContext(), 0, false);
CanonicalDD->setBody(EmptyBody);

assert(DD->isUserProvided());
assert(!DD->isDeleted());
}
}
}

C.recordClassMemberSpecHash(ToComplete, MemberSpecHash);
return SetAndSucceed(Result, makeReflection(ToComplete));
}
Expand Down
17 changes: 17 additions & 0 deletions clang/lib/Sema/SemaReflect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ class MetaActionsImpl : public MetaActions {

// Iterate over member specs.
unsigned AnonMemCtr = 0;
bool AtLeastOneMemberIsNonTriviallyConstr = false;

for (TagDataMemberSpec *MemberSpec : MemberSpecs) {
// Build the member declaration.
unsigned DiagID;
Expand Down Expand Up @@ -574,11 +576,26 @@ class MetaActionsImpl : public MetaActions {
S.Context.getSizeType(), DefinitionLoc);
}

if (const CXXRecordDecl *RD = MemberSpec->Ty->getAsCXXRecordDecl()) {
if (RD->hasUserDeclaredConstructor()) {
AtLeastOneMemberIsNonTriviallyConstr = true;
}
}

VirtSpecifiers VS;
S.ActOnCXXMemberDeclarator(&ClsScope, MemberAS, MemberDeclarator, MTP,
BitWidthCE, VS, ICIS_NoInit);
}

const bool NeedToGenerateDefaultConstr =
IncompleteDecl->getTagKind() == TagTypeKind::Union &&
AtLeastOneMemberIsNonTriviallyConstr &&
NewDecl->needsImplicitDefaultConstructor();

if (NeedToGenerateDefaultConstr) {
S.DeclareImplicitDefaultConstructor(NewDecl);
Copy link
Author

@BaLiKfromUA BaLiKfromUA Nov 23, 2024

Choose a reason for hiding this comment

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

potentially hacky approach but I tried to reuse existing code as much as possible to not copy generation of constructors

}

// Finish the member-specification and the class definition.
S.ActOnFinishCXXMemberSpecification(&ClsScope, NewDecl->getBeginLoc(),
NewDecl, SourceLocation{},
Expand Down
46 changes: 46 additions & 0 deletions libcxx/test/std/experimental/reflection/define-aggregate.pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
//
// RUN: %{exec} %t.exe > %t.stdout

#include <type_traits>
#include <experimental/meta>

#include <print>
Expand Down Expand Up @@ -303,4 +304,49 @@ static_assert(is_type(define_aggregate(^^S2, {

} // namespace repeat_calls

namespace non_trivial_constructor_and_destructor_of_union_members {
// https://github.com/bloomberg/clang-p2996/issues/115

// all members of union are trivially constructible and destructible
union U;
static_assert(is_type(define_aggregate(
^^U,
{
data_member_spec(^^int),
data_member_spec(^^char),
})));

static_assert(std::is_default_constructible<U>::value == true);
static_assert(std::is_trivially_constructible<U>::value == true);

static_assert(std::is_destructible<U>::value == true);
static_assert(std::is_trivially_destructible<U>::value == true);

// at least one member of union has non-trivial constructor or destructor
struct A {
constexpr A() {
// no-op
}

constexpr ~A() {
// no-op
}
};

union UU;
static_assert(is_type(define_aggregate(
^^UU,
{
data_member_spec(^^int),
data_member_spec(^^A),
})));

static_assert(std::is_default_constructible<UU>::value == true);
static_assert(std::is_trivially_constructible<UU>::value == false);

static_assert(std::is_destructible<UU>::value == true);
static_assert(std::is_trivially_destructible<UU>::value == false);

} // namespace non_trivial_constructor_and_destructor

int main() { }
Loading