Author: Nico Weber Date: 2023-08-07T15:48:23-04:00 New Revision: 0342bbf223fa12701a0570a23f9eac433b8b341c
URL: https://github.com/llvm/llvm-project/commit/0342bbf223fa12701a0570a23f9eac433b8b341c DIFF: https://github.com/llvm/llvm-project/commit/0342bbf223fa12701a0570a23f9eac433b8b341c.diff LOG: Revert "Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`." This reverts commit bddaa35177861545fc329123e55b6a3b34549507. Reverting as requested at https://reviews.llvm.org/D155895#4566945 (for breaking tests on Windows). 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 d926cfcd4d8665..3ef68ca8af6685 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -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 diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 699b315269deb3..be53ce3e472659 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -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) @@ -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(); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 6c29efc03a4b78..2c2c9dc8147437 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -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> @@ -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; } - } } } diff --git a/clang/test/SemaCXX/attr-trivial-abi.cpp b/clang/test/SemaCXX/attr-trivial-abi.cpp index accac42c25cf5f..c215f90eb124ce 100644 --- a/clang/test/SemaCXX/attr-trivial-abi.cpp +++ b/clang/test/SemaCXX/attr-trivial-abi.cpp @@ -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}} @@ -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 - _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
