lukasza updated this revision to Diff 543594.
lukasza marked an inline comment as done.
lukasza added a comment.
Addressed feedback from @gribozavr2
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155895/new/
https://reviews.llvm.org/D155895
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
@@ -45,6 +45,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include <map>
#include <optional>
@@ -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 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
