[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 marked 2 inline comments as done.
ahatanak added inline comments.


================
Comment at: include/clang/Sema/Sema.h:2126
+    NTCUC_AutoVar,
+    // Initializer expression for object with automatic storage duration.
+    NTCUC_AutoObjInit,
----------------
rjmccall wrote:
> 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?
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.


================
Comment at: lib/Sema/SemaDecl.cpp:11085
+    if (isa<ImplicitValueInitExpr>(I))
+      continue;
+    if (auto E = dyn_cast<InitListExpr>(I))
----------------
rjmccall wrote:
> Why is this okay?  Don't we need to check default-initialization for these?
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753