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.
1480d173fca1c83809ed5ac350c19021c7036a99
Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.
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]] S2 {
S2(S2&& other) {}
S2& operator=(S2&& other) { return *this; }
~S2() {}
union { Trivial field; };
};
static_assert(__is_trivially_relocatable(S2), "");
Before the fix Clang would warn that 'trivial_abi' is disallowed on 'S2'
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.
f1ba3de456dd2b5561c5ef78ddeea345066f8400
.
Repository:
rG LLVM Github Monorepo
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]] S1 {
+ S1(S1&& other) {}
+ S1& operator=(S1&& other) { return *this; }
+ ~S1() {}
+ Trivial field;
+ };
+ static_assert(__is_trivially_relocatable(S1), "");
+
+ // `S2` is like `S1`, but `field` is wrapped in an anonymous union (and it
+ // seems that trivial relocatability of `S1` and `S2` 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]] S2 {
+ S2(S2&& other) {}
+ S2& operator=(S2&& other) { return *this; }
+ ~S2() {}
+ union { Trivial field; };
+ };
+ static_assert(__is_trivially_relocatable(S2), "");
+
+ // `S3` is like `S2` but uses an anonymous `struct` rather than an anonymous
+ // `union.
+ struct [[clang::trivial_abi]] S3 {
+ S3(S3&& other) {}
+ S3& operator=(S3&& other) { return *this; }
+ ~S3() {}
+ struct { Trivial field; };
+ };
+ static_assert(__is_trivially_relocatable(S3), "");
+
+ // `S4` is like `S2` but with `[[clang::trivial_abi]]` also applied to the
+ // anonymous union.
+ //
+ // `S2` and `S3` examples above show that `[[clang::trivial_abi]]`
+ // *implicitly* propagates into anonymous union and structs. 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]] S4 {
+ S4(S4&& other) {}
+ S4& operator=(S4&& other) { return *this; }
+ ~S4() {}
+ 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(S4), "");
+
+ // Like `S2`, but the field of the anonymous union is *not* trivial.
+ struct [[clang::trivial_abi]] S5 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{trivial_abi' is disallowed on 'S5' because it has a field of a non-trivial class type}}
+ S5(S5&& other) {}
+ S5& operator=(S5&& other) { return *this; }
+ ~S5() {}
+ union {
+ Nontrivial field;
+ };
+ };
+ static_assert(!__is_trivially_relocatable(S5), "");
+
+} // 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,30 @@
}
}
- 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.
- QualType FT = FD->getType();
- if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) {
- PrintDiagAndRemoveAttr(4);
- return;
- }
-
- if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>())
- if (!RT->isDependentType() &&
- !cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
- PrintDiagAndRemoveAttr(5);
+ std::queue<const RecordDecl *> RecordsWithFieldsToCheck;
+ RecordsWithFieldsToCheck.push(&RD);
+ while (!RecordsWithFieldsToCheck.empty()) {
+ for (const FieldDecl *FD : RecordsWithFieldsToCheck.front()->fields()) {
+ // Ill-formed if the field is an ObjectiveC pointer or of a type that is
+ // non-trivial for the purpose of calls.
+ QualType FT = FD->getType();
+ if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) {
+ PrintDiagAndRemoveAttr(4);
return;
}
+
+ if (const auto *RT =
+ FT->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
+ if (RT->getDecl()->isAnonymousStructOrUnion()) {
+ RecordsWithFieldsToCheck.push(RT->getDecl());
+ } else if (!RT->isDependentType() &&
+ !cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
+ PrintDiagAndRemoveAttr(5);
+ return;
+ }
+ }
+ }
+ RecordsWithFieldsToCheck.pop();
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
