Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.

[lukasza]

Summary

This is a reland of https://reviews.llvm.org/D155895.

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]] S {
  S(S&& other) {}
  S& operator=(S&& other) { return *this; }
  ~S() {}
  union { Trivial field; };
};
static_assert(__is_trivially_relocatable(S), "");

Before the fix Clang would warn that 'trivial_abi' is disallowed on 'S'
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 that contain
fields with a non-trivial copy constructors or move constructors.

[lukasza]

The delta from https://reviews.llvm.org/D155895 is in clang/test/SemaCXX/attr-trivial-abi.cpp where 1) a comment has been added above the non-trivial move constructor of the Trivial test helper and 2) test expectations have been tweaked to account for _WIN64 and __MINGW32__ (cargo-culting these conditions from the already-existing tests in the top part of this test file).

I have considered making Windows and non-Windows behaving in the same way, which would require making Trivial trivially-copyable (and consequently trivially-movable). This wouldn't repro the original problem, so I abandoned this direction.

[lukasza]

@gribozavr2 - ping?

clang/include/clang/Basic/LangOptions.h

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

clang/lib/Frontend/CompilerInvocation.cpp

     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)

         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();

clang/lib/Sema/SemaDeclCXX.cpp

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

     }
   }
 
-  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;
       }
+    }
   }
 }
 

clang/test/SemaCXX/attr-trivial-abi.cpp

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

 static_assert(__is_trivially_relocatable(S20), "");
 #endif
 } // namespace deletedCopyMoveConstructor
+
+namespace anonymousUnionsAndStructs {
+  // Test helper:
+  struct [[clang::trivial_abi]] Trivial {
+    Trivial() {}
+
+    // Non-trivial move constructor and move assignment operator are defined
+    // below primarily because (in a subset of tests below) we want to hit the
+    // following scenario from https://en.cppreference.com/w/cpp/language/union:
+    //
+    // > If a union contains a non-static data member with a non-trivial special
+    // > member function (copy/move constructor, copy/move assignment, or
+    // > destructor), that function is deleted by default in the union and needs
+    // > to be defined explicitly by the programmer.
+    //
+    // (Note that explicit definition of a move constructor is impossible for
+    // an *anonymous* union.)
+    //
+    // Hitting this scenario is require to repro warnings like:
+    //
+    // * 'trivial_abi' cannot be applied to '(unnamed union}`
+    //   copy constructors and move constructors are all deleted
+    //   (this repros regardless of the presence of the fix / CLANG_ABI_COMPAT).
+    // * 'trivial_abi' cannot be applied to 'StructWithAnonymousUnion'
+    //   'trivial_abi' is disallowed 'StructWithAnonymousUnion' because it has a
+    //   field of a non-trivial class type
+    //   (repros only before the fix - `CLANG_ABI_COMPAT <= 17`)
+    //
+    // OTOH, hitting this scenario means that `Trivial` is not trivially
+    // copyable.  Sadly, this means that `__is_trivially_relocatable` asserts
+    // need different expectations on Windows/MSVC, where to be
+    // trivial-for-calls, an object must be trivially copyable.  (And it is only
+    // trivially relocatable, currently, if it is trivial for calls.)
+    Trivial(Trivial&& other) {}
+    Trivial& operator=(Trivial&& other) { return *this; }
+
+    ~Trivial() {}
+  };
+#if !defined(_WIN64) || defined(__MINGW32__)
+  static_assert(__is_trivially_relocatable(Trivial), "");
+#else
+  static_assert(!__is_trivially_relocatable(Trivial), "");
+#endif
+
+  // 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:
+#if !defined(_WIN64) || defined(__MINGW32__)
+  struct [[clang::trivial_abi]] BasicStruct {
+#else
+  struct [[clang::trivial_abi]] BasicStruct { // expected-warning {{'trivial_abi' cannot be applied to 'BasicStruct'}} expected-note {{trivial_abi' is disallowed on 'BasicStruct' because it has a field of a non-trivial class type}}
+#endif
+    BasicStruct(BasicStruct&& other) {}
+    BasicStruct& operator=(BasicStruct&& other) { return *this; }
+    ~BasicStruct() {}
+    Trivial field;
+  };
+#if !defined(_WIN64) || defined(__MINGW32__)
+  static_assert(__is_trivially_relocatable(BasicStruct), "");
+#else
+  static_assert(!__is_trivially_relocatable(BasicStruct), "");
+#endif
+
+  // `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`.
+#if (defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17) || \
+    (defined(_WIN64) && !defined(__MINGW32__))
+  struct [[clang::trivial_abi]] StructWithAnonymousUnion { // expected-warning {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnion'}} expected-note {{trivial_abi' is disallowed on 'StructWithAnonymousUnion' because it has a field of a non-trivial class type}}
+#else
+  struct [[clang::trivial_abi]] StructWithAnonymousUnion {
+#endif
+    StructWithAnonymousUnion(StructWithAnonymousUnion&& other) {}
+    StructWithAnonymousUnion& operator=(StructWithAnonymousUnion&& other) { return *this; }
+    ~StructWithAnonymousUnion() {}
+    union { Trivial field; };
+  };
+#if (defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17) || \
+    (defined(_WIN64) && !defined(__MINGW32__))
+  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).
+#if !defined(_WIN64) || defined(__MINGW32__)
+  struct [[clang::trivial_abi]] StructWithAnonymousStruct {
+#else
+  struct [[clang::trivial_abi]] StructWithAnonymousStruct { // expected-warning {{'trivial_abi' cannot be applied to 'StructWithAnonymousStruct'}} expected-note {{trivial_abi' is disallowed on 'StructWithAnonymousStruct' because it has a field of a non-trivial class type}}
+#endif
+    StructWithAnonymousStruct(StructWithAnonymousStruct&& other) {}
+    StructWithAnonymousStruct& operator=(StructWithAnonymousStruct&& other) { return *this; }
+    ~StructWithAnonymousStruct() {}
+    struct { Trivial field; };
+  };
+#if !defined(_WIN64) || defined(__MINGW32__)
+  static_assert(__is_trivially_relocatable(StructWithAnonymousStruct), "");
+#else
+  static_assert(!__is_trivially_relocatable(StructWithAnonymousStruct), "");
+#endif
+
+  // `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()`).
+#if (defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17) || \
+    (defined(_WIN64) && !defined(__MINGW32__))
+  struct [[clang::trivial_abi]] TrivialAbiAttributeAppliedToAnonymousUnion { // expected-warning {{'trivial_abi' cannot be applied to 'TrivialAbiAttributeAppliedToAnonymousUnion'}} expected-note {{trivial_abi' is disallowed on 'TrivialAbiAttributeAppliedToAnonymousUnion' because it has a field of a non-trivial class type}}
+#else
+  struct [[clang::trivial_abi]] TrivialAbiAttributeAppliedToAnonymousUnion {
+#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) || \
+    (defined(_WIN64) && !defined(__MINGW32__))
+  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 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnionWithNonTrivialField'}} expected-note {{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
+