[clang] bddaa35 - Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 7 11:30:55 PDT 2023
Author: Dmitri Gribenko
Date: 2023-08-07T20:30:48+02:00
New Revision: bddaa35177861545fc329123e55b6a3b34549507
URL: https://github.com/llvm/llvm-project/commit/bddaa35177861545fc329123e55b6a3b34549507
DIFF: https://github.com/llvm/llvm-project/commit/bddaa35177861545fc329123e55b6a3b34549507.diff
LOG: Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.
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.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D155895
Added:
Modified:
clang/include/clang/Basic/LangOptions.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/attr-trivial-abi.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 3ef68ca8af6685..d926cfcd4d8665 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -230,6 +230,12 @@ class LangOptions : public LangOptionsBase {
/// - consider classes with defaulted special member functions non-pod.
Ver15,
+ /// Attempt to be ABI-compatible with code generated by Clang 17.0.x.
+ /// This causes clang to:
+ /// - Treat `[[clang::trivial_abi]]` as ill-formed and ignore it if any
+ /// field is an anonymous union and/or struct.
+ Ver17,
+
/// Conform to the underlying platform's C and C++ ABIs as closely
/// as we can.
Latest
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index be53ce3e472659..699b315269deb3 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3474,6 +3474,8 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts,
GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "14.0");
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver15)
GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "15.0");
+ else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver17)
+ GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "17.0");
if (Opts.getSignReturnAddressScope() ==
LangOptions::SignReturnAddressScopeKind::All)
@@ -3959,6 +3961,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
Opts.setClangABICompat(LangOptions::ClangABI::Ver14);
else if (Major <= 15)
Opts.setClangABICompat(LangOptions::ClangABI::Ver15);
+ else if (Major <= 17)
+ Opts.setClangABICompat(LangOptions::ClangABI::Ver17);
} else if (Ver != "latest") {
Diags.Report(diag::err_drv_invalid_value)
<< A->getAsString(Args) << A->getValue();
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 2c2c9dc8147437..6c29efc03a4b78 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/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 "llvm/Support/SaveAndRestore.h"
#include <map>
@@ -10413,21 +10414,35 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) {
}
}
- 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 (getLangOpts().getClangABICompat() > LangOptions::ClangABI::Ver17 &&
+ 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;
}
+ }
}
}
diff --git a/clang/test/SemaCXX/attr-trivial-abi.cpp b/clang/test/SemaCXX/attr-trivial-abi.cpp
index c215f90eb124ce..accac42c25cf5f 100644
--- a/clang/test/SemaCXX/attr-trivial-abi.cpp
+++ b/clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -fclang-abi-compat=17 -verify %s -std=c++11 -DCLANG_ABI_COMPAT=17
void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
@@ -169,3 +170,115 @@ static_assert(!__is_trivially_relocatable(S20), "");
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 {
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17
+ // expected-warning at -2 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnion'}}
+ // expected-note at -3 {{trivial_abi' is disallowed on 'StructWithAnonymousUnion' because it has a field of a non-trivial class type}}
+#endif
+ StructWithAnonymousUnion(StructWithAnonymousUnion&& other) {}
+ StructWithAnonymousUnion& operator=(StructWithAnonymousUnion&& other) { return *this; }
+ ~StructWithAnonymousUnion() {}
+ union { Trivial field; };
+ };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17
+ static_assert(!__is_trivially_relocatable(StructWithAnonymousUnion), "");
+#else
+ static_assert(__is_trivially_relocatable(StructWithAnonymousUnion), "");
+#endif
+
+ // `StructWithAnonymousStruct` is like `StructWithAnonymousUnion` but uses an
+ // anonymous `struct` rather than an anonymous `union. The same expectations
+ // can be applied to CLANG_ABI_COMPAT <= 17 and 18+, because the anonymous
+ // `struct` does have move constructors in the test below (unlike the
+ // anonymous `union` in the previous `StructWithAnonymousUnion` test).
+ 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 {
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17
+ // expected-warning at -2 {{'trivial_abi' cannot be applied to 'TrivialAbiAttributeAppliedToAnonymousUnion'}}
+ // expected-note at -3 {{trivial_abi' is disallowed on 'TrivialAbiAttributeAppliedToAnonymousUnion' because it has a field of a non-trivial class type}}
+#endif
+ 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;
+ };
+ };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17
+ static_assert(!__is_trivially_relocatable(TrivialAbiAttributeAppliedToAnonymousUnion), "");
+#else
+ static_assert(__is_trivially_relocatable(TrivialAbiAttributeAppliedToAnonymousUnion), "");
+#endif
+
+ // Like `StructWithAnonymousUnion`, but the field of the anonymous union is
+ // *not* trivial.
+ struct [[clang::trivial_abi]] StructWithAnonymousUnionWithNonTrivialField {
+ // expected-warning at -1 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnionWithNonTrivialField'}}
+ // expected-note at -2 {{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
+
More information about the cfe-commits
mailing list