[clang] 0342bbf - Revert "Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`."

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 7 12:48:49 PDT 2023


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 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