[cfe-dev] Fwd: Review of transparent_union patch

[Eli Friedman]

On Fri, Jan 23, 2009 at 11:33 AM, Anders Johnsen <skabet at gmail.com> wrote:
> Hi,
>
> Thank you for the review. I've attached a corrected patch.

A few notes:

1. This breaks CodeGen for transparent unions, which was working
before; it seems like this will be a large regression for CodeGen for
many programs.

2. The way the attribute is getting attached looks wrong; this is a
union attribute, so it's required to appear immediately after the
definition of the union.  I suppose that's mostly orthogonal to this
patch, though.

3.
+  // Ignore anonymous unions.
+  if (RD->isAnonymousStructOrUnion()) {
+    S.Diag(Attr.getLoc(),
+        diag::warn_transparent_union_attribute_anonymous);
+    return;
+  }
Why exactly are we ignoring anonymous structs/unions?  gcc seems to
support them; here's a testcase:
void a(union {int a; int* b;} __attribute((transparent_union)) x);
void b() {a(1);}

4. It looks like this allows floating-point types where gcc doesn't; testcase:

union x {float a;int* b;} __attribute((transparent_union));
void a(union x a);
void b() {a(1.0f);}

-Eli