Re: Recent go changes broke alpha bootstrap

[Ian Taylor <iant at golang dot org>]

On Thu, Oct 30, 2014 at 5:46 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> On Thu, Oct 30, 2014 at 11:11:09AM +0100, Uros Bizjak wrote:
>> On Thu, Oct 30, 2014 at 8:40 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> > Recent go changes broke alpha bootstrap:
>>
>> > $files/space/homedirs/uros/gcc-svn/trunk/libgo/go/os/stat_atim.go:22:29:
>> > error: reference to undefined field or method âMtimâ
>> >    modTime: timespecToTime(st.Mtim),
>> >                              ^
>> > /space/homedirs/uros/gcc-svn/trunk/libgo/go/os/stat_atim.go:60:50:
>> > error: reference to undefined field or method âAtimâ
>> >   return timespecToTime(fi.Sys().(*syscall.Stat_t).Atim)
>
> Hrm, this code relies on the old way of converting unions to Go
> structures which, replaced the union with its first element
> converted to a Go type.  I.e. the resulting size of the Go
> representation of the union was wrong.
>
> So, there was a kind hack that allowed to address the first field
> of an anonymous union as if it was a plain field of the
> surrounding structure (similar to C), and existing code now relies
> on that.

Yes, I was trying to remember if there was a case that needed this
approach, but I couldn't remember one.  We do need to handle anonymous
unions specially.



> 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.

Ian