Skip to content

Commit

Permalink
Revert "Anonymous unions should be transparent wrt `[[clang::trivial_…
Browse files Browse the repository at this point in the history 
…abi]]`."

This reverts commit bddaa35.
Reverting as requested at https://reviews.llvm.org/D155895#4566945
(for breaking tests on Windows).
  • Loading branch information
@nico
nico committed Aug 7, 2023
1 parent 9e99a4f commit 0342bbf
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 142 deletions.
6 changes: 0 additions & 6 deletions clang/include/clang/Basic/LangOptions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,6 @@ 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
Expand Down
4 changes: 0 additions & 4 deletions clang/lib/Frontend/CompilerInvocation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3474,8 +3474,6 @@ 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)
Expand Down Expand Up @@ -3961,8 +3959,6 @@ 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();
Expand Down
23 changes: 4 additions & 19 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#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>
Expand Down Expand Up @@ -10414,35 +10413,21 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) {
}
}

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.
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>()) {
// 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 (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>())
if (!RT->isDependentType() &&
!cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) {
PrintDiagAndRemoveAttr(5);
return;
}
}
}
}

Expand Down
113 changes: 0 additions & 113 deletions clang/test/SemaCXX/attr-trivial-abi.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// 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}}

Expand Down Expand Up @@ -170,115 +169,3 @@ 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@-2 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnion'}}
// expected-note@-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@-2 {{'trivial_abi' cannot be applied to 'TrivialAbiAttributeAppliedToAnonymousUnion'}}
// expected-note@-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@-1 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnionWithNonTrivialField'}}
// expected-note@-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

0 comments on commit 0342bbf

Please sign in to comment.