[Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[ahatanak]

Summary

This patch diagnoses uses of non-trivial C unions in the following contexts:

• function parameters
• function returns
• variables with automatic storage duration
• assignment
• compound literal with automatic storage duration
• block capture except when a __block variable is captured by a non-escaping block

Note that we already reject non-trivial C structs/unions used for variable function arguments. Also this patch doesn't detect non-trivial C unions passed to functions without function prototypes. I think that's okay since clang will reject the function definition's parameters, but if it isn't okay, it's possible to add diagnostics for that too.

See the discussion in https://reviews.llvm.org/D62988 for more background.

rdar://problem/50679094

[ahatanak]

This patch should diagnose lvalue-to-rvalue conversion of volatile non-trivial C unions too, but it currently doesn't.

void test(volatile union U0 *a) {
  (void)*a;
}

[rjmccall]

include/clang/Basic/DiagnosticSemaKinds.td

633

Should this be "`%select{contains|is}2 a union that is non-trivial to..."? None of these situations bans non-trivial types, just unions containing them (and aggregates containing such unions).

include/clang/Sema/Sema.h

2116

If this is parallel with the diagnostic, I'd suggest mentioning that in the comment.

2121

Same typo in both of these.

2126

Please expand this comment to clarify that this it's only used when we (might be) initializing an object by copying another.

Why is this specific to automatic storage duration? Just because C doesn't allow other initializers to be non-constant in the first place?

lib/Sema/SemaDecl.cpp

11085

Why is this okay? Don't we need to check default-initialization for these?

11089

Please include a comment like:

// Assume all other initializers involving copying some existing object.
// TODO: ignore any initializers where we can guarantee copy-elision

[ahatanak]

include/clang/Sema/Sema.h

2126

Yes, that was my reasoning. We want to reject copy initializations of non-trivial C unions with automatic storage duration since the compiler doesn't know how to do the copy, but I don't think variables with static duration have that problem since the initializers are constant, which enables them to be initialized according to the rules described in the C standard.

lib/Sema/SemaDecl.cpp

11085

I didn't consider an ImplicitValueInitExpr to be a default-initialization since IRGen currently emits a memcpy to initialize the field instead of calling a synthesized default-initialization function as it does when a local variable that's a non-trivial C struct is declared without an initializer.

typedef struct {
  id f0, f1;
} S0 ;

typedef struct {
  S0 s0;
  int f2;
} S;

void test(void) {
  // emits a memcpy of a global constant.
  S s = { .f2 = 1 };
}

It's not clear to me whether this is a default-initialization, but if it is, we should check default-initialization here and IRGen should be fixed to emit a call to the default-initialization function.

[rjmccall]

lib/Sema/SemaDecl.cpp

11085

C does not use the terms "default-initialization" and "value-initialization", but the the initialization performed for static/thread storage duration objects generally matches what C++ would call value-initialization. These rules are laid out in section 6.7.9 of the standard, which also includes the following in its description of the rules for list-initializations of aggregates (regardless of their storage duration):

p19: ...all subobjects that are not initialized explicitly shall be initialized implicitly the same as objects that have static storage duration.

So s.s0 must be initialized as if a static object (in C++ terms, value-initialized), not left with an indeterminate value like an object of automatic storage duration without an initializer would be (in C++ terms, default-initialized).

I assume that the default-initialization function for a non-trivial C struct only initializes the fields that have non-trivial default-initialization. That's not good enough for value-initialization, which is required to recursively value-initialize *all* of the fields (and even zero-initialize any padding). Note that that's the *only* difference between these two kinds of initialization. Furthermore, if you think about what default-initialization actually means for all our leaf non-trivial C types, it's just assigning a predictable bit-pattern (e.g. a null pointer for ARC's __strong and __weak references), not doing anything that requires executing tailored code at runtime. So here's what that tells us:

• We can't call the default-initialization function because it might leave trivial fields uninitialized (in general, although struct S0 in your example doesn't have any, so effectively it does a full value-initialization).

• memcpy from a global constant is always a valid implementation of value-initialization for non-trivial C types. It's also a valid implementation for default-implementation; it just might do more than it really needs to. (In the case of struct S0, it would be *more efficient* to use memset, but memcpy is still *valid*.)
  
  Now, maybe someday we'll add a non-trivial C type that really requires non-trivial code to initialize, but I doubt we'd want to, because we'd have to forbid e.g. declaring global variables of that type, and we wouldn't have a simple answer for telling people how to properly initialize a dynamically-allocated object. Even if we do, we can wait until then to generalize.

• Regardless, yes, this is a value-initialization and so it should check whether we can value-initialize the type. Since value-initialization just means default-initialization plus a bunch of zero-initialization (which we can assume we can always do), that just means checking whether we can default-initialize the type.

[ahatanak]

Address review comments. Diagnose lvalue-to-rvalue conversion of volatile non-trivial C types.

[ahatanak]

include/clang/Basic/DiagnosticSemaKinds.td

633

Yes, that's correct. We only ban aggregates containing non-trivial unions (the former case) and non-trivial unions (the latter case).

[rjmccall]

include/clang/Sema/Sema.h

2124

"default-initialized", please.

lib/Sema/SemaExpr.cpp

16218

It looks like you're generally warning about this based on the specific context that forced an lvalue-to-rvalue conversion. I'm not sure volatile is special except that we actually perform the load even in unused-value contexts. Is the assumption that you've exhaustively covered all the other contexts of lvalue-to-rvalue conversions whose values will actually be used? That seems questionable to me.

[ahatanak]

lib/Sema/SemaExpr.cpp

16218

Yes, that was my assumption. All the other contexts where lvalue-to-rvalue conversion is performed and the result is used are already covered by other calls sites of checkNonTrivialCUnion, which informs the users that the struct/union is being used in an invalid context.

Do you have a case in mind that I didn't think of where a lvalue-to-rvalue conversion requires a non-trivial initialization/destruction/copying of a union but clang fails to emit any diagnostics?

Also I realized that lvalue-to-rvalue conversion of volatile types doesn't always require non-trivial destruction, so I think CheckDestruct shouldn't be set in this case.

void foo(U0 *a, volatile U0 *b) {
  // this doesn't require destruction.
  // this is perfectly valid if U0 is non-trivial to destruct but trivial to copy.
  *a = *b;  
}

For the same reason, I think CheckDestruct shouldn't be set for function returns (but it should be set for function parameters since they are destructed by the callee).

[rjmccall]

lib/Sema/SemaExpr.cpp

16218

There are a *lot* of places that trigger lvalue-to-rvalue conversion. Many of them aren't legal with structs (in C), but I'm worried about approving a pattern with the potential to be wrong by default just because we didn't think about some weird case. As an example, casts can trigger lvalue-to-rvalue conversion; I think the only casts allowed with structs are the identity cast and the cast to void, but those are indeed allowed. Now, a cast to void means the value is ignored, so we can elide a non-volatile load in the operand, and an identity cast isn't terminal; if the idea is that we're checking all the *terminal* uses of a struct r-value, then we're in much more restricted territory (and don't need to worry about things like commas and conditional operators that can propagate values out). But this still worries me.

I'm not sure we need to be super-pedantic about destructibility vs. copyability in some of this checking. It's certain possible in C++, but I can't imagine what sort of *primitive* type would be trivially copyable but not trivially destructible. (The reverse isn't true: something like a relative pointer would be non-trivially copyable but still trivially destructible.)

Is there any way to make this check cheaper so that we can immediately avoid any further work for types that are obviously copyable/destructible? All the restricted types are (possibly arrays of) record types, right?

[ahatanak]

lib/Sema/SemaExpr.cpp

16218

I'm not sure I fully understand what you are saying, but by "cheaper", do you mean fewer and simpler rules for allowing or disallowing non-trivial unions even when that might result in rejecting unions used in contexts in which non-trivial initialization/destruction/copying is not required? If so, we can probably diagnose any lvalue-to-rvalue conversions regardless of whether the source is volatile if the type is either non-trivial to copy or destruct.

[rjmccall]

lib/Sema/SemaExpr.cpp

16218

Sorry, that point was separate from the discussion of volatile and lvalue-to-rvalue conversions. I mean that you're changing a lot of core paths in Sema, and it would be nice if we could very quickly decide based on the type that no restrictions apply instead of having to make a function call, a switch, and a bunch of other calls in order to realize that e.g. void* never needs additional checking. Currently you have a !CPlusPlus check in front of all the checkNonTrivialCUnion calls; I would like something that reliably avoids doing this work for the vast majority of types that are not restricted, even in C.

[ahatanak]

lib/Sema/SemaExpr.cpp

16218

Instead of checking !CPlusPlus, we can call isNonTrivialPrimitiveCType (which is deleted in this patch) to do a cheaper check of whether the type requires any non-trivial default-initialize/destruct/copy functions. The function still uses the copy visitor, so if we want to make the check even cheaper, we can add a flag to RecordDecl that indicates whether it contains a non-trivial union.

[rjmccall]

lib/Sema/SemaExpr.cpp

16218

I think it would be sufficient for the fast path to just check for a C record type (i.e. !isa<CXXRecordDecl>(RT->getDecl())). But it doesn't seem unreasonable to record non-copyable/destructible/defaultable types on the RecordDecl if we have the bits.

[ahatanak]

Call hasNonTrivialPrimitiveCStruct to check whether the type is a non-trivial C struct/union before calling checkNonTrivialCUnion. Fix comments.

[ahatanak]

lib/Sema/SemaExpr.cpp

16218

I don't think we can call !isa<CXXRecordDecl> if the type is an array? I created a new function hasNonTrivialPrimitiveCStruct instead that checks whether the type is a non-trivial C struct or union.

[ahatanak]

lib/Sema/SemaExpr.cpp

16218

Actually, it's not wrong. It only excludes C++ records, so the function call will be made for all the other types including C structs/unions or arrays of them that are trivial.

[rjmccall]

include/clang/AST/Type.h

1132

You only want these checks to trigger on unions with non-trivial members (or structs containing them), right? How about something like hasNonTrivialPrimitiveCUnionMember()? Or maybe make it more descriptive for the use sites, like isPrimitiveCRestrictedType()?

Also, it would be nice if the fast path of this could be inlined so that clients usually didn't had to make a call at all. You can write the getBaseElementTypeUnsafe()->getAs<RecordType>() part in an inline implementation at the bottom this file.

lib/Sema/SemaExpr.cpp

16218

I was skipping a few steps, but yes, you'd need to look through array types (which you can do "unsafely" because you don't need to care about top-level qualification), then check if the base element type was a RecordType.

[ahatanak]

include/clang/AST/Type.h

1132

Since we don't keep track of whether a struct or union is or contains unions with non-trivial members, we'll have to use the visitors to detect such structs or unions or, to do it faster, add a bit to RecordDeclBits that indicates the presence of non-trivial unions. I guess it's okay to add another bit to RecordDeclBits?

[rjmccall]

include/clang/AST/Type.h

1132

It looks like there's plenty of space in RecordDeclBits, yeah.

[ahatanak]

Add a bit to RecordDeclBitfields that indicates whether the record has a non-trivial C union member.

[rjmccall]

Thanks, that looks great. A few more requests and then this will be ready to go, I think.

include/clang/AST/DeclBase.h

1454 ↗

(On Diff #208861)

This needs to be updated.

include/clang/AST/Type.h

1132

This comment seems like the right place to explain what makes a union non-trivial in C (that it contains a member which is non-trivial for *any* of the reasons that a type might be non-trivial).

1235

This should be static, right?

lib/Sema/SemaDecl.cpp

11623

Please add a comment explaining why this is specific to local variables.

12026–12027

Please add a comment explaining why this is specific to local variables.

lib/Sema/SemaExpr.cpp

6097

Can we extract a common function that checks the initializer expression of a non-trivial C union? I think there are at least three separate places that do that in this patch, and it's not too hard to imagine that we might want to add more cases to the common analysis.

[ahatanak]

lib/Sema/SemaExpr.cpp

6097

Do you mean we should have something like bool checkNonTrivialCUnionInInitList(const Expr *Init) so that we don't have to call dyn_cast? If Init is an InitListExpr, the function diagnoses non-trivial C unions types and returns true, otherwise it returns false.

[rjmccall]

lib/Sema/SemaExpr.cpp

6097

Well, the goal is not to eliminate a dyn_cast, it's to generally perform whatever checking is necessary for an initializer. Currently all of our checking is specific to InitListExpr, but there might be other cases we'll want to care about, in which case we'll probably care about them in all three of these places.

[ahatanak]

Address review comments.

[ahatanak]

lib/Sema/SemaDecl.cpp

12026–12027

I was trying to explain why this should be specific to local variables and realized that it's not clear to me whether it should be.

Suppose there is a union with two fields that are both non-trivial:

union U {
  Type A a;
  Type B a;
};

U global;

In this case, is value-initialization (which is essentially default-initialization plus a bunch of zero-initialization as per our previous discussion) used to initialize global? If so, should we reject the code since it requires default-initialization? It should be fine if we can assume default-initialization means zero-initialization for non-trivial types in C, but what if TypeA or TypeB requires initializing to a non-zero value?

[rjmccall]

lib/Sema/SemaDecl.cpp

12026–12027

Yeah, the default-initialization dimension of this problem is interesting. The C++ rule makes sense for C++ because default initialization of a C++ class requires an actual, arbitrary-side-effects constructor call, which of course you can't reasonably do implicitly for a union member. As discussed previously, non-trivial C types can presumably always be default-initialized with a constant bit pattern. That means that, as long as we can do any initialization work at all, then it's in principle not a problem as long as the bit pattern is the same for all the union members requiring non-trivial initialization (and in particular if there's only one such member). So it's just like you say, we *could* just initialize such unions conservatively as long as two different members don't require inconsistent patterns, which in practice they currently never do. That's all true independent of storage duration — if we can write that pattern into a global, we can write into a local. The only caveat is that a semantic need for non-trivial default initialization almost certainly means that there's a semantic need for non-trivial destruction as well, which of course can't be done on a local union (but isn't a problem for a global because we just don't destroy them).

On the other hand, on a language level it's much simpler to just say that we can't default-initialize a union of any storage duration if it has a non-trivial member, and then the language rule doesn't depend on bit-level representations. If there's interest, we can look into weakening that rule later by saying that e.g. it's possible to default-initialize a union with at most one non-trivial member.

Apropos, do we consider unions with non-trivial members to be non-trivial members for the purposes of enclosing unions? Seems like we should. Probably the most sensible way to handle that is to also flag the union as being non-trivial in a dimension if it has a member that's non-trivial in that dimension (which might also let you fast-path some of the checking you need to do). Essentially, we'd consider the case where copying is impossible to be a subset of the case where copying is non-trivial.

[ahatanak]

Diagnose C union globals that are non-trivial to default-initialize. Add 3 bits to RecordDeclBitfields for default-initialize, destruct, and copy to fast-path checking.

lib/Sema/SemaDecl.cpp

12026–12027

Yes, this patch does consider unions with non-trivial members to be non-trivial members for the purposes of enclosing unions.

I've made changes that make clang diagnose global variables that are or have C union types that are non-trivial to default-initialize. This disallows declaring global C union variables that have ObjC ARC pointer fields, but we can relax this later if users want them.

[rjmccall]

Thanks, just a few minor comment requests now.

include/clang/AST/DeclBase.h

1453 ↗

(On Diff #209565)

Please include in these comments that these imply the associated basic non-triviality predicates.

include/clang/AST/Type.h

1132

Okay, if we're tracking these separately, please put separate comments on each. Also, please mention in each comment that this implies the associated basic non-triviality predicate.

lib/Sema/SemaDecl.cpp

12026–12027

Well, presumably you're only diagnosing them if they're actually default-initialized. Users have an easy workaround if they actually want to declare a global union containing a __strong reference: they can just initialize the member they actually want to initialize.

[ahatanak]

Improve comments.

[ahatanak]

In Type.h, move method declarations down and mention that the predicates imply the associated basic non-triviality predicates.

[rjmccall]

Thanks!

include/clang/AST/Type.h

enum PrimitiveCopyKind {
     /// PCK_Trivial nor PCK_VolatileTrivial.
     /// Note that a C++ struct type does not necessarily match this; C++ copying
     /// semantics are too complex to express here, in part because they depend
     /// on the exact constructor or assignment operator that is chosen by
     /// overload resolution to do the copy.
     PCK_Struct
   };
 
   /// Check if this is a non-trivial type that would cause a C struct
-  /// transitively containing this type to be non-trivial. This function can be
-  /// used to determine whether a field of this type can be declared inside a C
-  /// union.
-  bool isNonTrivialPrimitiveCType(const ASTContext &Ctx) const;
-
-  /// Check if this is a non-trivial type that would cause a C struct
   /// transitively containing this type to be non-trivial to copy and return the
   /// kind.
   PrimitiveCopyKind isNonTrivialToPrimitiveCopy() const;
 
   /// Check if this is a non-trivial type that would cause a C struct
   /// transitively containing this type to be non-trivial to destructively
   /// move and return the kind. Destructive move in this context is a C++-style
   /// move in which the source object is placed in a valid but unspecified state
private:
   static SplitQualType getSplitDesugaredType(QualType T);
   static SplitQualType getSplitUnqualifiedTypeImpl(QualType type);
   static QualType getSingleStepDesugaredTypeImpl(QualType type,
                                                  const ASTContext &C);
   static QualType IgnoreParens(QualType T);
   static DestructionKind isDestructedTypeImpl(QualType type);
 };
 
 } // namespace clang
 
 namespace llvm {
 
 /// Implement simplify_type for QualType, so that we can dyn_cast from QualType
 /// to a specific Type class.
 template<> struct simplify_type< ::clang::QualType> {
   using SimpleType = const ::clang::Type *;
 

include/clang/Basic/DiagnosticSemaKinds.td

def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
   InGroup<ImplicitFunctionDeclare>, DefaultError;
 def warn_cstruct_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this "
   "%1 call is a pointer to record %2 that is not trivial to "
   "%select{primitive-default-initialize|primitive-copy}3">,
   InGroup<NonTrivialMemaccess>;
 def note_nontrivial_field : Note<
   "field is non-trivial to %select{copy|default-initialize}0">;
-def err_nontrivial_primitive_type_in_union : Error<
-  "non-trivial C types are disallowed in union">;
+def err_non_trivial_c_union_in_invalid_context : Error<
+  "cannot %select{"
+  "use type %1 for a function/method parameter|"
+  "use type %1 for function/method return|"
+  "implicitly initialize an object of type %1|"
+  "declare an unitialized automatic variable of type %1|"
+  "declare an automatic variable of type %1|"
+  "initialize an object that has automatic storage duration of type %1|"
+  "assign to a variable of type %1|"
+  "construct an automatic compound literal of type %1|"
+  "capture a variable of type %1|"
+  "cannot use volatile type %1 where it causes an lvalue-to-rvalue conversion"
+  "}3 "
+  "since it %select{contains|is}2 a union that is non-trivial to "
+  "%select{default-initialize|destruct|copy}0">;
+def note_non_trivial_c_union : Note<
+  "%select{%2 has subobjects that are|%3 has type %2 that is}0 "
+  "non-trivial to %select{default-initialize|destruct|copy}1">;
 def warn_dyn_class_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this "
   "%1 call is a pointer to %select{|class containing a }2dynamic class %3; "
   "vtable pointer will be %select{overwritten|copied|moved|compared}4">,
   InGroup<DynamicClassMemaccess>;
 def note_bad_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
 def warn_sizeof_pointer_expr_memaccess : Warning<

include/clang/Sema/Sema.h

void ActOnParamDefaultArgument(Decl *param,
                                  Expr *defarg);
   void ActOnParamUnparsedDefaultArgument(Decl *param,
                                          SourceLocation EqualLoc,
                                          SourceLocation ArgLoc);
   void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
   bool SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
                                SourceLocation EqualLoc);
 
+  // Contexts where using non-trivial C union types can be disallowed. This is
+  // passed to err_non_trivial_c_union_in_invalid_context.
+  enum NonTrivialCUnionContext {
+    // Function parameter.
+    NTCUC_FunctionParam,
+    // Function return.
+    NTCUC_FunctionReturn,
+    // Implicitly initialized subobject.
+    NTCUC_ImplicitInitSubObject,
+    // Uninialized variable with automatic storage duration.
+    NTCUC_UninitAutoVar,
+    // Variable with automatic storage duration.
+    NTCUC_AutoVar,
+    // Initializer expression for object with automatic storage duration that
+    // might copy from another object.
+    NTCUC_AutoObjInit,
+    // Assignment.
+    NTCUC_Assignment,
+    // Compound literal.
+    NTCUC_CompoundLiteral,
+    // Block capture.
+    NTCUC_BlockCapture,
+    // lvalue-to-rvalue conversion of volatile type.
+    NTCUC_LValueToRValueVolatile,
+  };
+
+  /// Emit diagnostics if any of the initilizer expressions in the passed
+  /// InitListExpr copies a type that is or contains a C union type that is
+  /// non-trivial to copy.
+  void checkNonTrivialCUnionCopyInInitList(const InitListExpr *ILE);
+
+  /// Emit diagnostics if a non-trivial C union type or a struct that contains
+  /// a non-trivial C union is used in an invalid context.
+  void checkNonTrivialCUnion(QualType QT, SourceLocation Loc,
+                             NonTrivialCUnionContext UseContext);
+
   void AddInitializerToDecl(Decl *dcl, Expr *init, bool DirectInit);
   void ActOnUninitializedDecl(Decl *dcl);
   void ActOnInitializerError(Decl *Dcl);
 
   void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
   void ActOnCXXForRangeDecl(Decl *D);
   StmtResult ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,
                                         IdentifierInfo *Ident,

lib/AST/Type.cpp

 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/NestedNameSpecifier.h"
-#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"

 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
   return !Context.getLangOpts().ObjCAutoRefCount &&
          Context.getLangOpts().ObjCWeak &&
          getObjCLifetime() != Qualifiers::OCL_Weak;
 }
 
-namespace {
-// Helper class that determines whether this is a type that is non-trivial to
-// primitive copy or move, or is a struct type that has a field of such type.
-template <bool IsMove>
-struct IsNonTrivialCopyMoveVisitor
-    : CopiedTypeVisitor<IsNonTrivialCopyMoveVisitor<IsMove>, IsMove, bool> {
-  using Super =
-      CopiedTypeVisitor<IsNonTrivialCopyMoveVisitor<IsMove>, IsMove, bool>;
-  IsNonTrivialCopyMoveVisitor(const ASTContext &C) : Ctx(C) {}
-  void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT) {}
-
-  bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) {
-    if (const auto *AT = this->Ctx.getAsArrayType(QT))
-      return this->asDerived().visit(Ctx.getBaseElementType(AT));
-    return Super::visitWithKind(PCK, QT);
-  }
-
-  bool visitARCStrong(QualType QT) { return true; }
-  bool visitARCWeak(QualType QT) { return true; }
-  bool visitTrivial(QualType QT) { return false; }
-  // Volatile fields are considered trivial.
-  bool visitVolatileTrivial(QualType QT) { return false; }
-
-  bool visitStruct(QualType QT) {
-    const RecordDecl *RD = QT->castAs<RecordType>()->getDecl();
-    // We don't want to apply the C restriction in C++ because C++
-    // (1) can apply the restriction at a finer grain by banning copying or
-    //     destroying the union, and
-    // (2) allows users to override these restrictions by declaring explicit
-    //     constructors/etc, which we're not proposing to add to C.
-    if (isa<CXXRecordDecl>(RD))
-      return false;
-    for (const FieldDecl *FD : RD->fields())
-      if (this->asDerived().visit(FD->getType()))
-        return true;
-    return false;
-  }
-
-  const ASTContext &Ctx;
-};
-
-} // namespace
-
-bool QualType::isNonTrivialPrimitiveCType(const ASTContext &Ctx) const {
-  if (isNonTrivialToPrimitiveDefaultInitialize())
-    return true;
-  DestructionKind DK = isDestructedType();
-  if (DK != DK_none && DK != DK_cxx_destructor)
-    return true;
-  if (IsNonTrivialCopyMoveVisitor<false>(Ctx).visit(*this))
-    return true;
-  if (IsNonTrivialCopyMoveVisitor<true>(Ctx).visit(*this))
-    return true;
-  return false;
-}
-
 QualType::PrimitiveDefaultInitializeKind
 QualType::isNonTrivialToPrimitiveDefaultInitialize() const {
   if (const auto *RT =
           getTypePtr()->getBaseElementTypeUnsafe()->getAs<RecordType>())
     if (RT->getDecl()->isNonTrivialToPrimitiveDefaultInitialize())
       return PDIK_Struct;
 
   switch (getQualifiers().getObjCLifetime()) {

lib/Sema/Sema.cpp

if (CXXDestructorDecl *DD = RD->getDestructor()) {
       S.ResolveExceptionSpec(Loc, FPT);
     }
 }
 
 static void markEscapingByrefs(const FunctionScopeInfo &FSI, Sema &S) {
   // Set the EscapingByref flag of __block variables captured by
   // escaping blocks.
   for (const BlockDecl *BD : FSI.Blocks) {
-    if (BD->doesNotEscape())
-      continue;
     for (const BlockDecl::Capture &BC : BD->captures()) {
       VarDecl *VD = BC.getVariable();
-      if (VD->hasAttr<BlocksAttr>())
+      if (VD->hasAttr<BlocksAttr>()) {
+        // Nothing to do if this is a __block variable captured by a
+        // non-escaping block.
+        if (BD->doesNotEscape())
+          continue;
         VD->setEscapingByref();
       }
+      // Check whether the captured variable is or contains an object of
+      // non-trivial C union type.
+      if (!S.getLangOpts().CPlusPlus)
+        S.checkNonTrivialCUnion(BC.getVariable()->getType(),
+                                BD->getCaretLocation(),
+                                Sema::NTCUC_BlockCapture);
+    }
   }
 
   for (VarDecl *VD : FSI.ByrefBlockVars) {
     // __block variables might require us to capture a copy-initializer.
     if (!VD->isEscapingByref())
       continue;
     // It's currently invalid to ever have a __block variable with an
     // array type; should we diagnose that here?

lib/Sema/SemaDecl.cpp

 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CommentDiagnostic.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/HeaderSearch.h" // TODO: Sema shouldn't depend on Lex
 #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering.
 #include "clang/Lex/ModuleLoader.h" // TODO: Sema shouldn't depend on Lex
if (!getLangOpts().CPlusPlus) {
     NewVD = VarDecl::Create(Context, DC, D.getBeginLoc(), D.getIdentifierLoc(),
                             II, R, TInfo, SC);
 
     if (R->getContainedDeducedType())
       ParsingInitForAutoVars.insert(NewVD);
 
     if (D.isInvalidType())
       NewVD->setInvalidDecl();
+
+    if (NewVD->hasLocalStorage())
+      checkNonTrivialCUnion(NewVD->getType(), NewVD->getLocation(),
+                            NTCUC_AutoVar);
   } else {
     bool Invalid = false;
 
     if (DC->isRecord() && !CurContext->isRecord()) {
       // This is an out-of-line definition of a static data member.
       switch (SC) {
       case SC_None:
         break;
if (isa<FunctionNoProtoType>(FT) && !D.isFunctionDefinition()) {
         // Windows system headers sometimes accidentally use stdcall without
         // (void) parameters, so we relax this to a warning.
         int DiagID =
             CC == CC_X86StdCall ? diag::warn_cconv_knr : diag::err_cconv_knr;
         Diag(NewFD->getLocation(), DiagID)
             << FunctionType::getNameForCallConv(CC);
       }
     }
+
+   checkNonTrivialCUnion(NewFD->getReturnType(),
+                         NewFD->getReturnTypeSourceRange().getBegin(),
+                         NTCUC_FunctionReturn);
   } else {
     // C++11 [replacement.functions]p3:
     //  The program's definitions shall not be specified as inline.
     //
     // N.B. We diagnose declarations instead of definitions per LWG issue 2340.
     //
     // Suppress the diagnostic if the function is __attribute__((used)), since
     // that forces an external definition to be emitted.
if (VarDecl *Old = VDecl->getPreviousDecl()) {
     MergeVarDeclTypes(VDecl, Old, /*MergeTypeWithPrevious*/ false);
   }
 
   // Check the deduced type is valid for a variable declaration.
   CheckVariableDeclarationType(VDecl);
   return VDecl->isInvalidDecl();
 }
 
+void Sema::checkNonTrivialCUnionCopyInInitList(const InitListExpr *ILE) {
+  for (auto I : ILE->inits()) {
+    if (isa<ImplicitValueInitExpr>(I))
+      checkNonTrivialCUnion(I->getType(), ILE->getExprLoc(),
+                            NTCUC_ImplicitInitSubObject);
+    else if (auto E = dyn_cast<InitListExpr>(I))
+      checkNonTrivialCUnionCopyInInitList(E);
+    else
+      // Assume all other initializers involving copying some existing object.
+      // TODO: ignore any initializers where we can guarantee copy-elision.
+      checkNonTrivialCUnion(I->getType(), I->getExprLoc(), NTCUC_AutoObjInit);
+  }
+};
+
+namespace {
+
+struct DiagNonTrivalCUnionDefaultInitializeVisitor
+    : DefaultInitializedTypeVisitor<DiagNonTrivalCUnionDefaultInitializeVisitor,
+                                    void> {
+  using Super =
+      DefaultInitializedTypeVisitor<DiagNonTrivalCUnionDefaultInitializeVisitor,
+                                    void>;
+
+  DiagNonTrivalCUnionDefaultInitializeVisitor(
+      QualType OrigTy, SourceLocation OrigLoc,
+      Sema::NonTrivialCUnionContext UseContext, Sema &S)
+      : OrigTy(OrigTy), OrigLoc(OrigLoc), UseContext(UseContext), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType QT,
+                     const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (const auto *AT = S.Context.getAsArrayType(QT))
+      return this->asDerived().visit(S.Context.getBaseElementType(AT), FD,
+                                     InNonTrivialUnion);
+    return Super::visitWithKind(PDIK, QT, FD, InNonTrivialUnion);
+  }
+
+  void visitARCStrong(QualType QT, const FieldDecl *FD,
+                      bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 0 << QT << FD->getName();
+  }
+
+  void visitARCWeak(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 0 << QT << FD->getName();
+  }
+
+  void visitStruct(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    const RecordDecl *RD = QT->castAs<RecordType>()->getDecl();
+    if (RD->isUnion()) {
+      if (OrigLoc.isValid()) {
+        bool IsUnion = false;
+        if (auto *OrigRD = OrigTy->getAsRecordDecl())
+          IsUnion = OrigRD->isUnion();
+        S.Diag(OrigLoc, diag::err_non_trivial_c_union_in_invalid_context)
+            << 0 << OrigTy << IsUnion << UseContext;
+        // Reset OrigLoc so that this diagnostic is emitted only once.
+        OrigLoc = SourceLocation();
+      }
+      InNonTrivialUnion = true;
+    }
+
+    if (InNonTrivialUnion)
+      S.Diag(RD->getLocation(), diag::note_non_trivial_c_union)
+          << 0 << 0 << QT.getUnqualifiedType() << "";
+
+    for (const FieldDecl *FD : RD->fields())
+      asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  }
+
+  void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {}
+
+  // The non-trivial C union type or the struct/union type that contains a
+  // non-trivial C union.
+  QualType OrigTy;
+  SourceLocation OrigLoc;
+  Sema::NonTrivialCUnionContext UseContext;
+  Sema &S;
+};
+
+struct DiagNonTrivalCUnionDestructedTypeVisitor
+    : DestructedTypeVisitor<DiagNonTrivalCUnionDestructedTypeVisitor, void> {
+  using Super =
+      DestructedTypeVisitor<DiagNonTrivalCUnionDestructedTypeVisitor, void>;
+
+  DiagNonTrivalCUnionDestructedTypeVisitor(
+      QualType OrigTy, SourceLocation OrigLoc,
+      Sema::NonTrivialCUnionContext UseContext, Sema &S)
+      : OrigTy(OrigTy), OrigLoc(OrigLoc), UseContext(UseContext), S(S) {}
+
+  void visitWithKind(QualType::DestructionKind DK, QualType QT,
+                     const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (const auto *AT = S.Context.getAsArrayType(QT))
+      return this->asDerived().visit(S.Context.getBaseElementType(AT), FD,
+                                     InNonTrivialUnion);
+    return Super::visitWithKind(DK, QT, FD, InNonTrivialUnion);
+  }
+
+  void visitARCStrong(QualType QT, const FieldDecl *FD,
+                      bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 1 << QT << FD->getName();
+  }
+
+  void visitARCWeak(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 1 << QT << FD->getName();
+  }
+
+  void visitStruct(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    const RecordDecl *RD = QT->castAs<RecordType>()->getDecl();
+    if (RD->isUnion()) {
+      if (OrigLoc.isValid()) {
+        bool IsUnion = false;
+        if (auto *OrigRD = OrigTy->getAsRecordDecl())
+          IsUnion = OrigRD->isUnion();
+        S.Diag(OrigLoc, diag::err_non_trivial_c_union_in_invalid_context)
+            << 1 << OrigTy << IsUnion << UseContext;
+        // Reset OrigLoc so that this diagnostic is emitted only once.
+        OrigLoc = SourceLocation();
+      }
+      InNonTrivialUnion = true;
+    }
+
+    if (InNonTrivialUnion)
+      S.Diag(RD->getLocation(), diag::note_non_trivial_c_union)
+          << 0 << 1 << QT.getUnqualifiedType() << "";
+
+    for (const FieldDecl *FD : RD->fields())
+      asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  }
+
+  void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {}
+  void visitCXXDestructor(QualType QT, const FieldDecl *FD,
+                          bool InNonTrivialUnion) {}
+
+  // The non-trivial C union type or the struct/union type that contains a
+  // non-trivial C union.
+  QualType OrigTy;
+  SourceLocation OrigLoc;
+  Sema::NonTrivialCUnionContext UseContext;
+  Sema &S;
+};
+
+struct DiagNonTrivalCUnionCopyVisitor
+    : CopiedTypeVisitor<DiagNonTrivalCUnionCopyVisitor, false, void> {
+  using Super = CopiedTypeVisitor<DiagNonTrivalCUnionCopyVisitor, false, void>;
+
+  DiagNonTrivalCUnionCopyVisitor(QualType OrigTy, SourceLocation OrigLoc,
+                                 Sema::NonTrivialCUnionContext UseContext,
+                                 Sema &S)
+      : OrigTy(OrigTy), OrigLoc(OrigLoc), UseContext(UseContext), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT,
+                     const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (const auto *AT = S.Context.getAsArrayType(QT))
+      return this->asDerived().visit(S.Context.getBaseElementType(AT), FD,
+                                     InNonTrivialUnion);
+    return Super::visitWithKind(PCK, QT, FD, InNonTrivialUnion);
+  }
+
+  void visitARCStrong(QualType QT, const FieldDecl *FD,
+                      bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 2 << QT << FD->getName();
+  }
+
+  void visitARCWeak(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    if (InNonTrivialUnion)
+      S.Diag(FD->getLocation(), diag::note_non_trivial_c_union)
+          << 1 << 2 << QT << FD->getName();
+  }
+
+  void visitStruct(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {
+    const RecordDecl *RD = QT->castAs<RecordType>()->getDecl();
+    if (RD->isUnion()) {
+      if (OrigLoc.isValid()) {
+        bool IsUnion = false;
+        if (auto *OrigRD = OrigTy->getAsRecordDecl())
+          IsUnion = OrigRD->isUnion();
+        S.Diag(OrigLoc, diag::err_non_trivial_c_union_in_invalid_context)
+            << 2 << OrigTy << IsUnion << UseContext;
+        // Reset OrigLoc so that this diagnostic is emitted only once.
+        OrigLoc = SourceLocation();
+      }
+      InNonTrivialUnion = true;
+    }
+
+    if (InNonTrivialUnion)
+      S.Diag(RD->getLocation(), diag::note_non_trivial_c_union)
+          << 0 << 2 << QT.getUnqualifiedType() << "";
+
+    for (const FieldDecl *FD : RD->fields())
+      asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  }
+
+  void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT,
+                const FieldDecl *FD, bool InNonTrivialUnion) {}
+  void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {}
+  void visitVolatileTrivial(QualType QT, const FieldDecl *FD,
+                            bool InNonTrivialUnion) {}
+
+  // The non-trivial C union type or the struct/union type that contains a
+  // non-trivial C union.
+  QualType OrigTy;
+  SourceLocation OrigLoc;
+  Sema::NonTrivialCUnionContext UseContext;
+  Sema &S;
+};
+
+} // namespace
+
+void Sema::checkNonTrivialCUnion(QualType QT, SourceLocation Loc,
+                                 NonTrivialCUnionContext UseContext) {
+  bool CheckInit = false, CheckDestruct = false, CheckCopy = false;
+
+  switch (UseContext) {
+  case NTCUC_FunctionParam:
+    CheckDestruct = CheckCopy = true;
+    break;
+  case NTCUC_FunctionReturn:
+    CheckDestruct = CheckCopy = true;
+    break;
+  case NTCUC_ImplicitInitSubObject:
+    CheckInit = true;
+    break;
+  case NTCUC_UninitAutoVar:
+    CheckInit = true;
+    break;
+  case NTCUC_AutoVar:
+    CheckDestruct = true;
+    break;
+  case NTCUC_AutoObjInit:
+    CheckCopy = true;
+    break;
+  case NTCUC_Assignment:
+    CheckCopy = true;
+    break;
+  case NTCUC_CompoundLiteral:
+    CheckDestruct = true;
+    break;
+  case NTCUC_BlockCapture:
+    CheckDestruct = CheckCopy = true;
+    break;
+  case NTCUC_LValueToRValueVolatile:
+    CheckDestruct = CheckCopy = true;
+    break;
+  }
+
+  if (CheckInit &&
+      QT.isNonTrivialToPrimitiveDefaultInitialize() == QualType::PDIK_Struct)
+    DiagNonTrivalCUnionDefaultInitializeVisitor(QT, Loc, UseContext, *this)
+        .visit(QT, nullptr, false);
+  if (CheckDestruct &&
+      QT.isDestructedType() == QualType::DK_nontrivial_c_struct)
+    DiagNonTrivalCUnionDestructedTypeVisitor(QT, Loc, UseContext, *this)
+        .visit(QT, nullptr, false);
+  if (CheckCopy && QT.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct)
+    DiagNonTrivalCUnionCopyVisitor(QT, Loc, UseContext, *this)
+        .visit(QT, nullptr, false);
+}
+
 /// AddInitializerToDecl - Adds the initializer Init to the
 /// declaration dcl. If DirectInit is true, this is C++ direct
 /// initialization rather than copy initialization.
 void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   // If there is no declaration, there was an error parsing it.  Just ignore
   // the initializer.
   if (!RealDecl || RealDecl->isInvalidDecl()) {
     CorrectDelayedTyposInExpr(Init, dyn_cast_or_null<VarDecl>(RealDecl));
// constant expressions.
       const Expr *Culprit;
       if (!Init->isConstantInitializer(Context, false, &Culprit)) {
         Diag(Culprit->getExprLoc(),
              diag::ext_aggregate_init_not_constant)
           << Culprit->getSourceRange();
       }
     }
 
+    if (!getLangOpts().CPlusPlus && VDecl->hasLocalStorage()) {
+      if (auto *ILE = dyn_cast<InitListExpr>(Init))
+        checkNonTrivialCUnionCopyInInitList(ILE);
+      else
+        checkNonTrivialCUnion(VDecl->getType(), VDecl->getLocation(),
+                              NTCUC_AutoObjInit);
+    }
+
     if (auto *E = dyn_cast<ExprWithCleanups>(Init))
       if (auto *BE = dyn_cast<BlockExpr>(E->getSubExpr()->IgnoreParens()))
         if (VDecl->hasLocalStorage())
           BE->getBlockDecl()->setCanAvoidCopyToHeap();
   } else if (VDecl->isStaticDataMember() && !VDecl->isInline() &&
              VDecl->getLexicalDeclContext()->isRecord()) {
     // This is an in-class initialization for a static data member, e.g.,
     //
if (getLangOpts().CPlusPlus && Var->hasLocalStorage()) {
             = Context.getBaseElementType(Type)->getAs<RecordType>()) {
         CXXRecordDecl *CXXRecord = cast<CXXRecordDecl>(Record->getDecl());
         // Mark the function (if we're in one) for further checking even if the
         // looser rules of C++11 do not require such checks, so that we can
         // diagnose incompatibilities with C++98.
         if (!CXXRecord->isPOD())
           setFunctionHasBranchProtectedScope();
       }
+    } else if (!getLangOpts().CPlusPlus && Var->hasLocalStorage()) {
+      checkNonTrivialCUnion(Var->getType(), Var->getLocation(),
+                            NTCUC_UninitAutoVar);
     }
+
     // In OpenCL, we can't initialize objects in the __local address space,
     // even implicitly, so don't synthesize an implicit initializer.
     if (getLangOpts().OpenCL &&
         Var->getType().getAddressSpace() == LangAS::opencl_local)
       return;
     // C++03 [dcl.init]p9:
     //   If no initializer is specified for an object, and the
     //   object is of (possibly cv-qualified) non-POD class type (or
if (getLangOpts().ObjCAutoRefCount &&
     }
     T = Context.getLifetimeQualifiedType(T, lifetime);
   }
 
   ParmVarDecl *New = ParmVarDecl::Create(Context, DC, StartLoc, NameLoc, Name,
                                          Context.getAdjustedParameterType(T),
                                          TSInfo, SC, nullptr);
 
+  if (!getLangOpts().CPlusPlus)
+    checkNonTrivialCUnion(New->getType(), New->getLocation(),
+                          NTCUC_FunctionParam);
+
   // Parameters can not be abstract class types.
   // For record types, this is done by the AbstractClassUsageDiagnoser once
   // the class has been completely parsed.
   if (!CurContext->isRecord() &&
       RequireNonAbstractType(NameLoc, T, diag::err_abstract_type_in_decl,
                              AbstractParamType))
     New->setInvalidDecl();
 
for (const auto *I : Record->decls()) {
         if (IFD->getDeclName())
           ++NumNamedMembers;
     }
   }
 
   // Verify that all the fields are okay.
   SmallVector<FieldDecl*, 32> RecFields;
 
-  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
        i != end; ++i) {
     FieldDecl *FD = cast<FieldDecl>(*i);
 
     // Get the type for the field.
     const Type *FDTy = FD->getType().getTypePtr();
 
     if (!FD->isAnonymousStructOrUnion()) {
if (FDTy->isFunctionType()) {
                                  AbstractIvarType)) {
         // Ivars can not have abstract class types
         FD->setInvalidDecl();
       }
       if (Record && FDTTy->getDecl()->hasObjectMember())
         Record->setHasObjectMember(true);
       if (Record && FDTTy->getDecl()->hasVolatileMember())
         Record->setHasVolatileMember(true);
-      if (Record && Record->isUnion() &&
-          FD->getType().isNonTrivialPrimitiveCType(Context))
-        Diag(FD->getLocation(),
-             diag::err_nontrivial_primitive_type_in_union);
     } else if (FDTy->isObjCObjectType()) {
       /// A field cannot be an Objective-c object
       Diag(FD->getLocation(), diag::err_statically_allocated_object)
         << FixItHint::CreateInsertion(FD->getLocation(), "*");
       QualType T = Context.getObjCObjectPointerType(FD->getType());
       FD->setType(T);
-    } else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
-               Record && !ObjCFieldLifetimeErrReported && Record->isUnion() &&
-               !getLangOpts().CPlusPlus) {
-      // It's an error in ARC or Weak if a field has lifetime.
-      // We don't want to report this in a system header, though,
-      // so we just make the field unavailable.
-      // FIXME: that's really not sufficient; we need to make the type
-      // itself invalid to, say, initialize or copy.
-      QualType T = FD->getType();
-      if (T.hasNonTrivialObjCLifetime()) {
-        SourceLocation loc = FD->getLocation();
-        if (getSourceManager().isInSystemHeader(loc)) {
-          if (!FD->hasAttr<UnavailableAttr>()) {
-            FD->addAttr(UnavailableAttr::CreateImplicit(Context, "",
-                          UnavailableAttr::IR_ARCFieldWithOwnership, loc));
-          }
-        } else {
-          Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
-            << T->isBlockPointerType() << Record->getTagKind();
-        }
-        ObjCFieldLifetimeErrReported = true;
-      }
     } else if (getLangOpts().ObjC &&
                getLangOpts().getGC() != LangOptions::NonGC &&
                Record && !Record->hasObjectMember()) {
       if (FD->getType()->isObjCObjectPointerType() ||
           FD->getType().isObjCGCStrong())
         Record->setHasObjectMember(true);
       else if (Context.getAsArrayType(FD->getType())) {
         QualType BaseType = Context.getBaseElementType(FD->getType());

lib/Sema/SemaExpr.cpp

Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
 
   if (isFileScope)
     if (auto ILE = dyn_cast<InitListExpr>(LiteralExpr))
       for (unsigned i = 0, j = ILE->getNumInits(); i != j; i++) {
         Expr *Init = ILE->getInit(i);
         ILE->setInit(i, ConstantExpr::Create(Context, Init));
       }
 
-  Expr *E = new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literalType,
+  auto *E = new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literalType,
                                               VK, LiteralExpr, isFileScope);
   if (isFileScope) {
     if (!LiteralExpr->isTypeDependent() &&
         !LiteralExpr->isValueDependent() &&
         !literalType->isDependentType()) // C99 6.5.2.5p3
       if (CheckForConstantInitializer(LiteralExpr, literalType))
         return ExprError();
   } else if (literalType.getAddressSpace() != LangAS::opencl_private &&
              literalType.getAddressSpace() != LangAS::Default) {
     // Embedded-C extensions to C99 6.5.2.5:
     //   "If the compound literal occurs inside the body of a function, the
     //   type name shall not be qualified by an address-space qualifier."
     Diag(LParenLoc, diag::err_compound_literal_with_address_space)
       << SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd());
     return ExprError();
   }
 
+  if (!isFileScope && !getLangOpts().CPlusPlus) {
+    // Compound literals that have automatic storage duration are destroyed at
+    // the end of the scope. Emit diagnostics if it is or contains a C union type
+    // that is non-trivial to destruct.
+    checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+                          NTCUC_CompoundLiteral);
+    if (auto ILE = dyn_cast<InitListExpr>(E->getInitializer()))
+      // Emit diagnostics if any of the initilizer expressions copies a type
+      // that is or contains a C union type that is non-trivial to copy.
+      checkNonTrivialCUnionCopyInInitList(ILE);
+  }
+
   return MaybeBindToTemporary(E);
 }
 
 ExprResult
 Sema::ActOnInitList(SourceLocation LBraceLoc, MultiExprArg InitArgList,
                     SourceLocation RBraceLoc) {
   // Immediately handle non-overload placeholders.  Overloads can be
   // resolved contextually, but everything else here can't.
if (!ResultTy.isNull()) {
       // // heap.
       // b();
 
       if (auto *BE = dyn_cast<BlockExpr>(RHS.get()->IgnoreParens()))
         if (auto *DRE = dyn_cast<DeclRefExpr>(LHS.get()->IgnoreParens()))
           if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
             if (VD->hasLocalStorage() && getCurScope()->isDeclScope(VD))
               BE->getBlockDecl()->setCanAvoidCopyToHeap();
+
+      if (!getLangOpts().CPlusPlus)
+        checkNonTrivialCUnion(LHS.get()->getType(), LHS.get()->getExprLoc(),
+                              NTCUC_Assignment);
     }
     RecordModifiableNonNullParam(*this, LHS.get());
     break;
   case BO_PtrMemD:
   case BO_PtrMemI:
     ResultTy = CheckPointerToMemberOperands(LHS, RHS, VK, OpLoc,
                                             Opc == BO_PtrMemI);
     break;
ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
 
   // Try to apply the named return value optimization. We have to check again
   // if we can do this, though, because blocks keep return statements around
   // to deduce an implicit return type.
   if (getLangOpts().CPlusPlus && RetTy->isRecordType() &&
       !BD->isDependentContext())
     computeNRVO(Body, BSI);
 
+  if (!getLangOpts().CPlusPlus)
+    checkNonTrivialCUnion(RetTy, BD->getCaretLocation(), NTCUC_FunctionReturn);
+
   PopDeclContext();
 
   // Pop the block scope now but keep it alive to the end of this function.
   AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
   PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);
 
   // Set the captured variables on the block.
   SmallVector<BlockDecl::Capture, 4> Captures;
default:
     break;
   }
 
   // Can't traverse through this node. Nothing to do.
   return ExprEmpty();
 }
 
 ExprResult Sema::CheckLValueToRValueConversionOperand(Expr *E) {
+  // Check whether the operand is or contains an object of non-trivial C union
+  // type.
+  if (E->getType().isVolatileQualified())
+    checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+                          Sema::NTCUC_LValueToRValueVolatile);
+
   // C++2a [basic.def.odr]p4:
   //   [...] an expression of non-volatile-qualified non-class type to which
   //   the lvalue-to-rvalue conversion is applied [...]
   if (E->getType().isVolatileQualified() || E->getType()->getAs<RecordType>())
     return E;
 
   ExprResult Result =
       rebuildPotentialResultsAsNonOdrUsed(*this, E, NOUR_Constant);

lib/Sema/SemaType.cpp

bool Sema::CheckFunctionReturnType(QualType T, SourceLocation Loc) {
   // Methods cannot return interface types. All ObjC objects are
   // passed by reference.
   if (T->isObjCObjectType()) {
     Diag(Loc, diag::err_object_cannot_be_passed_returned_by_value)
         << 0 << T << FixItHint::CreateInsertion(Loc, "*");
     return true;
   }
 
+  if (!getLangOpts().CPlusPlus)
+    checkNonTrivialCUnion(T, Loc, NTCUC_FunctionReturn);
+
   return false;
 }
 
 /// Check the extended parameter information.  Most of the necessary
 /// checking should occur when applying the parameter attribute; the
 /// only other checks required are positional restrictions.
 static void checkExtParameterInfos(Sema &S, ArrayRef<QualType> paramTypes,
                     const FunctionProtoType::ExtProtoInfo &EPI,

test/CodeGenObjC/Inputs/strong_in_union.h

-#ifndef STRONG_IN_UNION_H
-#define STRONG_IN_UNION_H
-#pragma clang system_header
-
-typedef union {
-  id f0;
-  int *f1;
-} U;
-
-#endif // STRONG_IN_UNION_H

test/CodeGenObjC/strong-in-c-struct.m

-// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -emit-llvm -o - -DUSESTRUCT -I %S/Inputs %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -emit-llvm -o - -DUSESTRUCT %s | FileCheck %s
 
-// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -emit-pch -I %S/Inputs -o %t %s
-// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -include-pch %t -emit-llvm -o - -DUSESTRUCT -I %S/Inputs %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -include-pch %t -emit-llvm -o - -DUSESTRUCT %s | FileCheck %s
 
 #ifndef HEADER
 #define HEADER
-#include "strong_in_union.h"
 
 typedef void (^BlockTy)(void);
 
 typedef struct {
   int a[4];
 } Trivial;
 
 typedef struct {

 // CHECK: %[[I64:.*]] = getelementptr inbounds %[[STRUCT_BITFIELD1]], %[[STRUCT_BITFIELD1]]* %[[V58]], i32 0, i32 9
 // CHECK: %[[V59:.*]] = load volatile i8, i8* %[[I64]], align 1
 // CHECK: store volatile i8 %[[V59]], i8* %[[I6]], align 1
 
 void test_copy_constructor_Bitfield1(Bitfield1 *a) {
   Bitfield1 t = *a;
 }
 
-// CHECK: define void @test_strong_in_union()
-// CHECK: alloca %{{.*}}
-// CHECK-NEXT: ret void
-
-void test_strong_in_union() {
-  U t;
-}
-
 // CHECK: define void @test_copy_constructor_VolatileArray(
 // CHECK: call void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE(
 
 // CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE(
 // CHECK: %[[ADDR_CUR:.*]] = phi i8**
 // CHECK: %[[ADDR_CUR1:.*]] = phi i8**
 // CHECK: %[[V12:.*]] = bitcast i8** %[[ADDR_CUR]] to i32*
 // CHECK: %[[V13:.*]] = bitcast i8** %[[ADDR_CUR1]] to i32*
 // CHECK: %[[V14:.*]] = load volatile i32, i32* %[[V13]], align 4
 // CHECK: store volatile i32 %[[V14]], i32* %[[V12]], align 4
 
 void test_copy_constructor_VolatileArray(VolatileArray *a) {
   VolatileArray t = *a;
 }
 
 #endif /* USESTRUCT */

test/SemaObjC/arc-decls.m

 // RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -verify -Wno-objc-root-class %s
 
 // rdar://8843524
 
 struct A {
   id x[4];
   id y;
 };
 
 union u {
-    id u; // expected-error {{ARC forbids Objective-C objects in union}}
-};
-
-union u_nontrivial_c {
-  struct A a; // expected-error {{non-trivial C types are disallowed in union}}
+  id u;
 };
 
 // Volatile fields are fine.
 struct C {
   volatile int x[4];
   volatile int y;
 };
 

test/SemaObjC/non-trivial-c-union.m

+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -verify %s
+
+typedef union { // expected-note 8 {{'U0' has subobjects that are non-trivial to default-initialize}} expected-note 36 {{'U0' has subobjects that are non-trivial to destruct}} expected-note 26 {{'U0' has subobjects that are non-trivial to copy}}
+  id f0; // expected-note 8 {{f0 has type '__strong id' that is non-trivial to default-initialize}} expected-note 36 {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note 26 {{f0 has type '__strong id' that is non-trivial to copy}}
+  __weak id f1; // expected-note 8 {{f1 has type '__weak id' that is non-trivial to default-initialize}} expected-note 36 {{f1 has type '__weak id' that is non-trivial to destruct}} expected-note 26 {{f1 has type '__weak id' that is non-trivial to copy}}
+} U0;
+
+typedef struct {
+  U0 f0;
+  id f1;
+} S0;
+
+id g0;
+U0 ug0;
+U0 ug1 = { .f0 = 0 };
+S0 sg0;
+S0 sg1 = { .f0 = {0}, .f1 = 0 };
+
+U0 foo0(U0); // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+S0 foo1(S0); // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to copy}}
+
+@interface C
+-(U0)m0:(U0)arg; // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+-(S0)m1:(S0)arg; // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to copy}}
+@end
+
+void testBlockFunction(void) {
+  (void)^(U0 a){ return ug0; }; // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+  (void)^(S0 a){ return sg0; }; // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to copy}}
+}
+void testAutoVar(void) {
+  U0 u0; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'U0' since it is a union that is non-trivial to default-initialize}}
+  U0 u1 = ug0; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot initialize an object that has automatic storage duration of type 'U0' since it is a union that is non-trivial to copy}}
+  U0 u2 = { g0 }; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}}
+  U0 u3 = { .f1 = g0 }; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}}
+  S0 s0; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'S0' since it contains a union that is non-trivial to default-initialize}}
+  S0 s1 = sg0; // expected-error {{declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot initialize an object that has automatic storage duration of type 'S0' since it contains a union that is non-trivial to copy}}
+  S0 s2 = { ug0 }; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot initialize an object that has automatic storage duration of type 'U0' since it is a union that is non-trivial to copy}}
+  S0 s3 = { .f0 = ug0 }; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot initialize an object that has automatic storage duration of type 'U0' since it is a union that is non-trivial to copy}}
+  S0 s4 = { .f1 = g0 }; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot implicitly initialize an object of type 'U0' since it is a union that is non-trivial to default-initialize}}
+}
+
+void testAssignment(void) {
+  ug0 = ug1; // expected-error {{cannot assign to a variable of type 'U0' since it is a union that is non-trivial to copy}}
+  sg0 = sg1; // expected-error {{cannot assign to a variable of type 'S0' since it contains a union that is non-trivial to copy}}
+}
+
+U0 ug2 = (U0){ .f1 = 0 };
+S0 sg2 = (S0){ .f0 = {0}, .f1 = 0 };
+
+void testCompoundLiteral(void) {
+  const U0 *t0 = &(U0){ .f0 = g0 }; // expected-error {{cannot construct an automatic compound literal of type 'U0' since it is a union that is non-trivial to destruct}}
+  const U0 *t1 = &(U0){ .f1 = g0 }; // expected-error {{cannot construct an automatic compound literal of type 'U0' since it is a union that is non-trivial to destruct}}
+  const S0 *t2 = &(S0){ .f0 = ug0 }; // expected-error {{cannot construct an automatic compound literal of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot initialize an object that has automatic storage duration of type 'U0' since it is a union that is non-trivial to copy}}
+  const S0 *t3 = &(S0){ .f1 = g0 }; // expected-error {{cannot construct an automatic compound literal of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot implicitly initialize an object of type 'U0' since it is a union that is non-trivial to default-initialize}}
+}
+
+typedef void (^BlockTy)(void);
+void escapingFunc(BlockTy);
+void noescapingFunc(__attribute__((noescape)) BlockTy);
+
+void testBlockCapture(void) {
+  U0 t0; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'U0' since it is a union that is non-trivial to default-initialize}}
+  S0 t1; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'S0' since it contains a union that is non-trivial to default-initialize}}
+  __block U0 t2; // expected-error {{cannot declare an automatic variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'U0' since it is a union that is non-trivial to default-initialize}}
+  __block S0 t3; // expected-error {{cannot declare an automatic variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot declare an unitialized automatic variable of type 'S0' since it contains a union that is non-trivial to default-initialize}}
+
+  escapingFunc(^{ g0 = t0.f0; }); // expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to copy}}
+  escapingFunc(^{ g0 = t1.f0.f0; }); // expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to copy}}
+  escapingFunc(^{ g0 = t2.f0; }); // expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to copy}}
+  escapingFunc(^{ g0 = t3.f0.f0; }); // expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to copy}}
+  noescapingFunc(^{ g0 = t0.f0; }); // expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'U0' since it is a union that is non-trivial to copy}}
+  noescapingFunc(^{ g0 = t1.f0.f0; }); // expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to destruct}} expected-error {{cannot capture a variable of type 'S0' since it contains a union that is non-trivial to copy}}
+  noescapingFunc(^{ g0 = t2.f0; });
+  noescapingFunc(^{ g0 = t3.f0.f0; });
+}
+
+void testVolatileLValueToRValue(volatile U0 *a) {
+  (void)*a; // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to destruct}} // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to copy}}
+}