tests fail with gccgo

[Michael Hudson-Doyle]

Like so:

----------------------------------------------------------------------
FAIL: encode_test.go:217: net_goyaml_test.TestMarshal.pN27_launchpad.net_goyaml_test.S

encode_test.go:221:
    c.Assert(string(data), Equals, item.data)
... obtained string = "" +
... "a: 1\n" +
... "b: 2\n"
... expected string = "" +
... "a: 1\n" +
... "b: 2\n" +
... "c: 3\n"

----------------------------------------------------------------------
FAIL: encode_test.go:238: net_goyaml_test.TestMarshalErrors.pN27_launchpad.net_goyaml_test.S

encode_test.go:241:
    c.Assert(err, ErrorMatches, item.error)
... value = nil
... regex string = "Duplicated key 'b' in struct struct \\{ B int; .*"
... Error value is nil

----------------------------------------------------------------------
FAIL: decode_test.go:364: net_goyaml_test.TestUnmarshal.pN27_launchpad.net_goyaml_test.S

decode_test.go:385:
    c.Assert(value, DeepEquals, item.value, Commentf("Item #%d", i))
... obtained *struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" } = &struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" }{A:1, C:goyaml_test.inlineB{B:2, inlineC:goyaml_test.inlineC{C:0}}}
... expected *struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" } = &struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" }{A:1, C:goyaml_test.inlineB{B:2, inlineC:goyaml_test.inlineC{C:3}}}
... Item #84

OOPS: 9 passed, 3 FAILED
--- FAIL: Test (0.02 seconds)
FAIL
FAIL launchpad.net/goyaml 0.025s

[Michael Hudson-Doyle (mwhudson)]

All failures seem to be something going wrong with ,inline.

[Michael Hudson-Doyle (mwhudson)]

it's a difference in how reflection handles unexported, inlined, struct fields. I filed a bug upstream: https://code.google.com/p/go/issues/detail?id=7247

I'm not sure which implementation gets it right!

[Mark Ramm (mark-ramm)]

This is marked as fixed in https://code.google.com/p/go/issues/detail?id=7247 can we test against that and see if all is now well?

[Michael Hudson-Doyle (mwhudson)]

Well, all is now well in the sense that the tests fail with go tip too; this bug report is indicating that the goyaml tests depended on a bug in the gc toolchain, which is what has been fixed.

I've just verified that the branch I proposed (now linked to this bug, sorry for not doing this before) fixes the tests on go tip as well as gccgo.

[Gustavo Niemeyer (niemeyer)]

Thanks Michael, I'll get that merged.

[Gustavo Niemeyer (niemeyer)]

Actually, I'm not quite sure that this is right. Exported fields of a non-exported anonymous field are in fact exported too. The test change simply makes everything public, ignoring this case.

I believe the original test should continue to work.

[Gustavo Niemeyer (niemeyer)]

Upstream discussion about this:

    http://golang.org/issue/7522

    https://groups.google.com/d/msg/golang-dev/1mW0fmtPSVU/p6cyiV1kAj4J

[Mark Ramm (mark-ramm)]

What's the next step on this?

What needs to happen to make the tests pass under GCC Go. If we get the right direction on this, I'm sure somebody from core can work on it -- but it would be great if I didn't have to pull them away from the many other issues that need attention for GCC to work to get this addressed.

[Gustavo Niemeyer (niemeyer)]

This will likely get fixed upstream. The change that went it to get gc and gccgo to work the same broke the behavior not only of goyaml, but also of the stock encoding/json package, and likely many other marshaling packages out there. There's already partial agreement from the core team that this is not okay and must be addressed before Go 1.3.

[Mark Ramm (mark-ramm)]

Awesome. Thanks for the update.

On Thu, Mar 13, 2014 at 1:29 PM, Gustavo Niemeyer <email address hidden>wrote:

> This will likely get fixed upstream. The change that went it to get gc
> and gccgo to work the same broke the behavior not only of goyaml, but
> also of the stock encoding/json package, and likely many other
> marshaling packages out there. There's already partial agreement from
> the core team that this is not okay and must be addressed before Go 1.3.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1275410
>
> Title:
> tests fail with gccgo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/goyaml/+bug/1275410/+subscriptions
>

[Tim Penhey (thumper)]

Tests still fail with gccgo 4.9 today on trusty.

[Gustavo Niemeyer (niemeyer)]

The "fix" for the problem, which made gc behave as gccgo, indeed broke
a lot of people and got reverted for 1.3. The right change would be to
fix gccgo at this point so that it works as gc, until upstream decides
how to fix it properly. It won't match the behavior of gccgo today.

On Wed, May 14, 2014 at 12:38 AM, Tim Penhey <email address hidden> wrote:
> Tests still fail with gccgo 4.9 today on trusty.
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1275410
>
> Title:
> tests fail with gccgo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/goyaml/+bug/1275410/+subscriptions

--

gustavo @ http://niemeyer.net