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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:12053
+                            NTCUC_UninitAutoVar);
     }
+
----------------
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Please add a comment explaining why this is specific to local variables.
> > 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?
> 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.
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.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63753/new/

https://reviews.llvm.org/D63753