[PATCH] D80055: Diagnose union tail padding

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.


================
Comment at: clang/test/CodeGen/union-tail-padding.c:28-36
+union Front {
+  int i;
+  long long ll;
+};
+
+union Front front1;
+union Front front2 = {};        // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
----------------
dexonsmith wrote:
> jfb wrote:
> > dexonsmith wrote:
> > > Are these warnings actionable?  What should users do when they see this warning?
> > Good point!
> > 
> > I was thinking about this, and was wondering if I should add a fixit which suggests using the first wider member of the union. The problem is to offer the same object representation... that's tricky on its own (there isn't always an exact match), and then we have to deal with type punning (in C++, not in C).
> > 
> > So I'd love ideas, because I'm not sure what to do. That being said, I wrote this partly because D68115 was surprising to folks, and partly because developers would like to opt-in to this diagnostic to find places where initialization isn't doing what they think.
> > 
> > Maybe instead we should suggest to leave uninitialized, and use an assignment, or `memset`?
> > Maybe instead we should suggest to leave uninitialized, and use an assignment, or `memset`?
> 
> It's not clear to me that those are better.  For example, `memset` doesn't seem right for non-PODs in C++.  I'm not sure what to suggest though.
I'd argue that non-PODs in unions don't seem right either ;-)
It would be easy to diagnose non-trivially-copyable types differently in this circumstance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80055