Skip to content

Commit

Permalink
Disallow trivial_abi on a class if all copy and move constructors are
Browse files Browse the repository at this point in the history
deleted

Instead of forcing the class to be passed in registers, which was what
r350920 did, issue a warning and inform the user that the attribute
cannot be used.

For more background, see this discussion:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190128/259907.html

This fixes PR39683.

rdar://problem/47308221

Differential Revision: https://reviews.llvm.org/D57626
  • Loading branch information
ahatanaka committed Jun 10, 2020
1 parent 661fcfc commit f466f0b
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 19 deletions.
1 change: 1 addition & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,7 @@ destroy the object before returning.
Attribute ``trivial_abi`` has no effect in the following cases:

- The class directly declares a virtual base or virtual methods.
- Copy constructors and move constructors of the class are all deleted.
- The class has a base class that is non-trivial for the purposes of calls.
- The class has a non-static data member whose type is non-trivial for the purposes of calls, which includes:

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -3301,6 +3301,12 @@ def err_attribute_output_parameter : Error<

def ext_cannot_use_trivial_abi : ExtWarn<
"'trivial_abi' cannot be applied to %0">, InGroup<IgnoredAttributes>;
def note_cannot_use_trivial_abi_reason : Note<
"'trivial_abi' is disallowed on %0 because %select{"
"its copy constructors and move constructors are all deleted|"
"it is polymorphic|"
"it has a base of a non-trivial class type|it has a virtual base|"
"it has a __weak field|it has a field of a non-trivial class type}1">;

// Availability attribute
def warn_availability_unknown_platform : Warning<
Expand Down
44 changes: 35 additions & 9 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9686,27 +9686,53 @@ void Sema::DiagnoseHiddenVirtualMethods(CXXMethodDecl *MD) {
}

void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) {
auto PrintDiagAndRemoveAttr = [&]() {
auto PrintDiagAndRemoveAttr = [&](unsigned N) {
// No diagnostics if this is a template instantiation.
if (!isTemplateInstantiation(RD.getTemplateSpecializationKind()))
if (!isTemplateInstantiation(RD.getTemplateSpecializationKind())) {
Diag(RD.getAttr<TrivialABIAttr>()->getLocation(),
diag::ext_cannot_use_trivial_abi) << &RD;
Diag(RD.getAttr<TrivialABIAttr>()->getLocation(),
diag::note_cannot_use_trivial_abi_reason) << &RD << N;
}
RD.dropAttr<TrivialABIAttr>();
};

// Ill-formed if the copy and move constructors are deleted.
auto HasNonDeletedCopyOrMoveConstructor = [&]() {
if (RD.needsImplicitCopyConstructor() &&
!RD.defaultedCopyConstructorIsDeleted())
return true;
if (RD.needsImplicitMoveConstructor() &&
!RD.defaultedMoveConstructorIsDeleted())
return true;
for (const CXXConstructorDecl *CD : RD.ctors())
if (CD->isCopyOrMoveConstructor() && !CD->isDeleted())
return true;
return false;
};

if (!HasNonDeletedCopyOrMoveConstructor()) {
PrintDiagAndRemoveAttr(0);
return;
}

// Ill-formed if the struct has virtual functions.
if (RD.isPolymorphic()) {
PrintDiagAndRemoveAttr();
PrintDiagAndRemoveAttr(1);
return;
}

for (const auto &B : RD.bases()) {
// Ill-formed if the base class is non-trivial for the purpose of calls or a
// virtual base.
if ((!B.getType()->isDependentType() &&
!B.getType()->getAsCXXRecordDecl()->canPassInRegisters()) ||
B.isVirtual()) {
PrintDiagAndRemoveAttr();
if (!B.getType()->isDependentType() &&
!B.getType()->getAsCXXRecordDecl()->canPassInRegisters()) {
PrintDiagAndRemoveAttr(2);
return;
}

if (B.isVirtual()) {
PrintDiagAndRemoveAttr(3);
return;
}
}
Expand All @@ -9716,14 +9742,14 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) {
// non-trivial for the purpose of calls.
QualType FT = FD->getType();
if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) {
PrintDiagAndRemoveAttr();
PrintDiagAndRemoveAttr(4);
return;
}

if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>())
if (!RT->isDependentType() &&
!cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
PrintDiagAndRemoveAttr();
PrintDiagAndRemoveAttr(5);
return;
}
}
Expand Down
51 changes: 41 additions & 10 deletions clang/test/SemaObjCXX/attr-trivial-abi.mm
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ struct __attribute__((trivial_abi)) S1 {
int a;
};

struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} expected-note {{has a __weak field}}
__weak id a;
};

struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
virtual void m();
};

struct S3_2 {
virtual void m();
} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}}
} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}

struct S4 {
int a;
};

struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
};

struct __attribute__((trivial_abi)) S9 : public S4 {
Expand All @@ -36,19 +36,19 @@ struct __attribute__((trivial_abi)) S9 : public S4 {
__weak id a;
};

struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} expected-note {{has a __weak field}}
__weak id a;
};

struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} expected-note {{has a __weak field}}
__weak id a[2];
};

struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} expected-note {{has a field of a non-trivial class type}}
S6 a;
};

struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} expected-note {{has a field of a non-trivial class type}}
S6 a[2];
};

Expand All @@ -66,7 +66,7 @@ struct __attribute__((trivial_abi)) S10 {
S10<__weak id> p2;

template<>
struct __attribute__((trivial_abi)) S10<id> { // expected-warning {{'trivial_abi' cannot be applied to 'S10<id>'}}
struct __attribute__((trivial_abi)) S10<id> { // expected-warning {{'trivial_abi' cannot be applied to 'S10<id>'}} expected-note {{has a __weak field}}
__weak id a;
};

Expand All @@ -90,8 +90,39 @@ struct __attribute__((trivial_abi)) S16 {
S16<int> s16;

template<class T>
struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
__weak id a;
};

S17<int> s17;

namespace deletedCopyMoveConstructor {
struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
CopyMoveDeleted(const CopyMoveDeleted &) = delete;
CopyMoveDeleted(CopyMoveDeleted &&) = delete;
};

struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
CopyMoveDeleted a;
};

struct __attribute__((trivial_abi)) CopyDeleted {
CopyDeleted(const CopyDeleted &) = delete;
CopyDeleted(CopyDeleted &&) = default;
};

struct __attribute__((trivial_abi)) MoveDeleted {
MoveDeleted(const MoveDeleted &) = default;
MoveDeleted(MoveDeleted &&) = delete;
};

struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{copy constructors and move constructors are all deleted}}
CopyDeleted a;
MoveDeleted b;
};

// This is fine since the move constructor isn't deleted.
struct __attribute__((trivial_abi)) S20 {
int &&a; // a member of rvalue reference type deletes the copy constructor.
};
}

0 comments on commit f466f0b

Please sign in to comment.