lukasza created this revision.
Herald added a project: All.
lukasza requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Consider the test input below:
struct [[clang::trivial_abi]] Trivial {
Trivial() {}
Trivial(Trivial&& other) {}
Trivial& operator=(Trivial&& other) { return *this; }
~Trivial() {}
};
static_assert(__is_trivially_relocatable(Trivial), "");
struct [[clang::trivial_abi]] S {
S(S&& other) {}
S& operator=(S&& other) { return *this; }
~S() {}
union { Trivial field; };
};
static_assert(__is_trivially_relocatable(S), "");
Before the fix Clang would warn that 'trivial_abi' is disallowed on 'S'
because it has a field of a non-trivial class type (the type of the
anonymous union is non-trivial, because it doesn't have the
`[[clang::trivial_abi]]` attribute applied to it). Consequently, before
the fix the `static_assert` about `__is_trivially_relocatable` would
fail.
Note that `[[clang::trivial_abi]]` cannot be applied to the anonymous
union, because Clang warns that 'trivial_abi' is disallowed on '(unnamed
union at ...)' because its copy constructors and move constructors are
all deleted. Also note that it is impossible to provide copy nor move
constructors for anonymous unions and structs.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D156003
Files:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/attr-trivial-abi.cpp
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===================================================================
--- clang/test/SemaCXX/attr-trivial-abi.cpp
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -169,3 +169,94 @@
static_assert(__is_trivially_relocatable(S20), "");
#endif
} // namespace deletedCopyMoveConstructor
+
+namespace anonymousUnionsAndStructs {
+ // Test helper:
+ struct [[clang::trivial_abi]] Trivial {
+ Trivial() {}
+ Trivial(Trivial&& other) {}
+ Trivial& operator=(Trivial&& other) { return *this; }
+ ~Trivial() {}
+ };
+ static_assert(__is_trivially_relocatable(Trivial), "");
+
+ // Test helper:
+ struct Nontrivial {
+ Nontrivial() {}
+ Nontrivial(Nontrivial&& other) {}
+ Nontrivial& operator=(Nontrivial&& other) { return *this; }
+ ~Nontrivial() {}
+ };
+ static_assert(!__is_trivially_relocatable(Nontrivial), "");
+
+ // Basic smoke test, not yet related to anonymous unions or structs:
+ struct [[clang::trivial_abi]] BasicStruct {
+ BasicStruct(BasicStruct&& other) {}
+ BasicStruct& operator=(BasicStruct&& other) { return *this; }
+ ~BasicStruct() {}
+ Trivial field;
+ };
+ static_assert(__is_trivially_relocatable(BasicStruct), "");
+
+ // `StructWithAnonymousUnion` is like `BasicStruct`, but `field` is wrapped in
+ // an anonymous union, and thus trivial relocatability of `BasicStruct` and
+ // `StructWithAnonymousUnion` should be the same).
+ //
+ // It's impossible to declare a constructor for an anonymous unions so to
+ // support applying `[[clang::trivial_abi]]` to structs containing anonymous
+ // unions, and therefore when processing fields of the struct containing the
+ // anonymous union, the trivial relocatability of the *union* is ignored and
+ // instead the union's fields are recursively inspected in
+ // `checkIllFormedTrivialABIStruct`.
+ struct [[clang::trivial_abi]] StructWithAnonymousUnion {
+ StructWithAnonymousUnion(StructWithAnonymousUnion&& other) {}
+ StructWithAnonymousUnion& operator=(StructWithAnonymousUnion&& other) { return *this; }
+ ~StructWithAnonymousUnion() {}
+ union { Trivial field; };
+ };
+ static_assert(__is_trivially_relocatable(StructWithAnonymousUnion), "");
+
+ // `StructWithAnonymousStruct` is like `StructWithAnonymousUnion` but uses an
+ // anonymous `struct` rather than an anonymous `union.
+ struct [[clang::trivial_abi]] StructWithAnonymousStruct {
+ StructWithAnonymousStruct(StructWithAnonymousStruct&& other) {}
+ StructWithAnonymousStruct& operator=(StructWithAnonymousStruct&& other) { return *this; }
+ ~StructWithAnonymousStruct() {}
+ struct { Trivial field; };
+ };
+ static_assert(__is_trivially_relocatable(StructWithAnonymousStruct), "");
+
+ // `TrivialAbiAttributeAppliedToAnonymousUnion` is like
+ // `StructWithAnonymousUnion` but with `[[clang::trivial_abi]]` also applied
+ // to the anonymous union.
+ //
+ // The example below shows that it is still *not* okay to explicitly apply
+ // `[[clang::trivial_abi]]` to anonymous unions. Handling this would require
+ // relaxing the `HasNonDeletedCopyOrMoveConstructor` check when
+ // `isAnonymousStructOrUnion` in `checkIllFormedTrivialABIStruct` but when
+ // that check runs `setAnonymousStructOrUnion` hasn't been called yet (i.e. at
+ // this point it's not possible to rely on `RD->isAnonymousStructOrUnion()`).
+ struct [[clang::trivial_abi]] TrivialAbiAttributeAppliedToAnonymousUnion {
+ TrivialAbiAttributeAppliedToAnonymousUnion(TrivialAbiAttributeAppliedToAnonymousUnion&& other) {}
+ TrivialAbiAttributeAppliedToAnonymousUnion& operator=(TrivialAbiAttributeAppliedToAnonymousUnion&& other) { return *this; }
+ ~TrivialAbiAttributeAppliedToAnonymousUnion() {}
+ union [[clang::trivial_abi]] { // expected-warning {{'trivial_abi' cannot be applied to '(unnamed union}} expected-note {{copy constructors and move constructors are all deleted}}
+ Trivial field;
+ };
+ };
+ static_assert(__is_trivially_relocatable(TrivialAbiAttributeAppliedToAnonymousUnion), "");
+
+ // Like `StructWithAnonymousUnion`, but the field of the anonymous union is
+ // *not* trivial.
+ struct [[clang::trivial_abi]] StructWithAnonymousUnionWithNonTrivialField { // expected-warning {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnionWithNonTrivialField'}} expected-note {{trivial_abi' is disallowed on 'StructWithAnonymousUnionWithNonTrivialField' because it has a field of a non-trivial class type}}
+ StructWithAnonymousUnionWithNonTrivialField(StructWithAnonymousUnionWithNonTrivialField&& other) {}
+ StructWithAnonymousUnionWithNonTrivialField& operator=(StructWithAnonymousUnionWithNonTrivialField&& other) { return *this; }
+ ~StructWithAnonymousUnionWithNonTrivialField() {}
+ union {
+ Nontrivial field;
+ };
+ };
+ static_assert(!__is_trivially_relocatable(StructWithAnonymousUnionWithNonTrivialField), "");
+
+} // namespace anonymousStructsAndUnions
+
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -48,6 +48,7 @@
#include "llvm/ADT/StringExtras.h"
#include <map>
#include <optional>
+#include <queue>
#include <set>
using namespace clang;
@@ -10321,21 +10322,34 @@
}
}
- for (const auto *FD : RD.fields()) {
- // Ill-formed if the field is an ObjectiveC pointer or of a type that is
- // non-trivial for the purpose of calls.
+ llvm::SmallVector<const FieldDecl *> FieldsToCheck{RD.fields()};
+ while (!FieldsToCheck.empty()) {
+ const FieldDecl *FD = FieldsToCheck.pop_back_val();
+
+ // Ill-formed if the field is an ObjectiveC pointer.
QualType FT = FD->getType();
if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) {
PrintDiagAndRemoveAttr(4);
return;
}
- if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>())
+ if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
+ // Check the fields of anonymous structs (and/or or unions) instead of
+ // checking the type of the anonynous struct itself.
+ if (RT->getDecl()->isAnonymousStructOrUnion()) {
+ FieldsToCheck.append(RT->getDecl()->field_begin(),
+ RT->getDecl()->field_end());
+ continue;
+ }
+
+ // Ill-formed if the field is of a type that is non-trivial for the
+ // purpose of calls.
if (!RT->isDependentType() &&
!cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
PrintDiagAndRemoveAttr(5);
return;
}
+ }
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
