On Tue, Nov 4, 2014 at 1:00 AM, Ian Taylor <i...@golang.org> wrote:
> On Mon, Nov 3, 2014 at 9:02 AM, Dominik Vogt <v...@linux.vnet.ibm.com> wrote:
>> On Thu, Oct 30, 2014 at 08:05:14AM -0700, Ian Taylor wrote:
>>> On Thu, Oct 30, 2014 at 5:46 AM, Dominik Vogt <v...@linux.vnet.ibm.com>
>>> wrote:
>>> > I'm not quite sure about the best approach. The attempt to
>>> > represent C unions in the "right" way is ultimately futile as Go
>>> > does not have the types necessary for it. For example, the
>>> > handling of anonymous bit fields will never be right as it's
>>> > undefinied. On the other hand I could fix the issue at hand by
>>> > changing the way anonymous unions are represented in Go.
>>> >
>>> > Example:
>>> >
>>> > struct { int8_t x; union { int16_t y; int 32_t z; }; };
>>> >
>>> > Was represented (before the patch) as
>>> >
>>> > struct { X byte; int16 Y; }
>>> >
>>> > which had size 4, alignment 2 and y at offset 2 but should had
>>> > have size 8, alignment 4 and y at offset 4. With the current
>>> > patch the Go layout is
>>> >
>>> > struct { X byte; artificial_name struct { y [2]byte; align [0]int32; } }
>>> >
>>> > with the proper size, alignment and offset, but y is addressed as
>>> > ".artificial_name.y" insted of just ".y", and y is a byte array
>>> > and not an int16.
>>> >
>>> > I could remove the "artificial_name struct" and add padding before
>>> > and after y instead:
>>> >
>>> > struct { X byte; pad_0 [3]byte; Y int16; pad_1 [2]byte; align [0]int32;
>>> > }
>>> >
>>> > What do you think?
>>>
>>> Sounds good to me. Basically the fields of the anonymous union should
>>> be promoted to become fields of the struct. We can't do it in
>>> general, but we can do it for the first field. That addresses the
>>> actual uses of anonymous unions.
>>
>> The attached patch fixes this, at least if the first element of the
>> union is not a bitfield.
>>
>> Bitfields can really not be represented properly in Go (think about
>> constructs like "struct { int : 1; int bf : 1; }"), I'd rather not
>> try to represent them in a predictable way. The patched code may
>> or may not give them a name, and reserves the proper amount of
>> padding where required in structs. If a union begins with an
>> anonymous bitfield (which makes no sense), that is ignored. If a
>> union begins with a named bitfield (possibly after unnamed ones),
>> this may or may not be used as the (sole) element of the Go
>> struct.
>
>
> Thanks. I committed your patch.
I have checked that the patch fixes alpha bootstrap with libgo.
Thanks,
Uros.
