3398 – Attributes inside a union screws data alignment

Here is a test case:
union Foo1
{
 int a;
 float b;
}
version = Test;
union Foo2
{
 version (Test) {
 int a;
 float b;
 }
}
pragma(msg, Foo1.sizeof.stringof); // 4u (okay)
pragma(msg, Foo2.sizeof.stringof); // 8u (wrong)

This applies not just to version declarations; it applies to any attribute.
union Foo1 {
 double[2] a;
 double[2] b; 
}
union Foo2 {
public:
 double[2] a;
 double[2] b; 
}
static assert(Foo1.sizeof == Foo2.sizeof);
---
bug.d(32): Error: static assert (16u == 32u) is false
Subtle and disastrous bug (especially when dealing with C libraries), escalating severity.
Here's a mitigation to create an error message, rather than generate wrong code.
MITIGATION: struct.c StructDeclaration::semantic() line 384.
The problem is that AttribDeclaration::semantic has it's own loop for
doing the semantic on members, which does NOT include setting the offset to 0
if it's a union. Fixing this would not be difficult, but would require a minor structural change in the compiler.
---
 for (int i = 0; i < members_dim; i++)
 {
 Dsymbol *s = (Dsymbol *)members->data[i];
+ if (isUnionDeclaration() && s->isAttribDeclaration())
+ error("Attributes don't yet work inside unions, see Bugzilla 3398");
 s->semantic(sc2);
 if (isUnionDeclaration())
 sc2->offset = 0;
-------

One instance where wrong code is currently generated is in 
std.c.windows.winsock.d, in_addr and in6_addr.
My mitigation would break existing code, though, in cases where the AttribDeclaration is actually an AnonDeclaration.

PATCH: Currently, after processing every field, sc->offset gets changed, and StructDeclaration::semantic() sets the offset back to zero if it was a union.
This patch moves the processing to AggregateDeclaration::addField() instead. If it's a union, don't change the offset in the first place.
Index: struct.c
===================================================================
--- struct.c	(revision 527)
+++ struct.c	(working copy)
@@ -194,20 +194,23 @@
 sizeok = 2; // cannot finish; flag as forward referenced
 return;
 }
+ size_t ofs = sc->offset;
 
 memsize = t->size(loc);
 memalignsize = t->alignsize();
 xalign = t->memalign(sc->structalign);
- alignmember(xalign, memalignsize, &sc->offset);
- v->offset = sc->offset;
- sc->offset += memsize;
- if (sc->offset > structsize)
- structsize = sc->offset;
+ alignmember(xalign, memalignsize, &ofs);
+ v->offset = ofs;
+ ofs += memsize;
+ if (ofs > structsize)
+ structsize = ofs;
 if (sc->structalign < memalignsize)
 memalignsize = sc->structalign;
 if (alignsize < memalignsize)
 alignsize = memalignsize;
 //printf("\talignsize = %d\n", alignsize);
+ if (!isUnionDeclaration())
+ sc->offset = ofs;
 
 v->storage_class |= STCfield;
 //printf(" addField '%s' to '%s' at offset %d, size = %d\n", v->toChars(), toChars(), v->offset, memsize);
@@ -386,8 +389,6 @@
 {
 Dsymbol *s = (Dsymbol *)members->data[i];
 s->semantic(sc2);
- if (isUnionDeclaration())
- sc2->offset = 0;
 #if 0
 if (sizeok == 2)
 { //printf("forward reference\n");

http://www.dsource.org/projects/dmd/changeset/529