code review 6866047: cmd/gc: fix wrong interaction between inlining and embe...

cmd/gc: fix wrong interaction between inlining and embedded builtins.

The patch makes the compile user an ordinary package-local
symbol for the name of embedded fields of builtin type.

This is incompatible with the fix delivered for issue 2687
(revision 3c060add43fb) but fixes it in a different way, because
the explicit symbol on the field makes the typechecker able to
find it in lookdot.

Fixes issue 3552.

[remyoudompheng, 2012-12-03 16:53:25]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[lvd2, 2012-12-03 17:51:19]

LGTM

[remyoudompheng, 2012-12-03 19:14:30]

Hello golang-dev@googlegroups.com, lvd@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[remyoudompheng, 2012-12-03 19:14:35]

https://codereview.appspot.com/6866047/diff/3/test/fixedbugs/issue3552.go
File test/fixedbugs/issue3552.go (right):

https://codereview.appspot.com/6866047/diff/3/test/fixedbugs/issue3552.go#new...
test/fixedbugs/issue3552.go:1: // $G $D/$F.dir/one.go && $G $D/$F.dir/two.go ||
echo BUG:bug434
I seem to have forgotten the header.

[lvd2, 2012-12-03 19:17:40]

https://codereview.appspot.com/6866047/diff/3/test/run.go
File test/run.go (right):

https://codereview.appspot.com/6866047/diff/3/test/run.go#newcode707
test/run.go:707: "fixedbugs/bug345.go":     true, // broken
this is unrelated?

[remyoudompheng, 2012-12-03 19:23:38]

https://codereview.appspot.com/6866047/diff/3/test/run.go
File test/run.go (right):

https://codereview.appspot.com/6866047/diff/3/test/run.go#newcode707
test/run.go:707: "fixedbugs/bug345.go":     true, // broken
yes, I'll revert the change (hoping we find a fix soon).

[remyoudompheng, 2012-12-03 19:23:52]

Hello golang-dev@googlegroups.com, lvd@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-12-06 05:58:55]

https://codereview.appspot.com/6866047/diff/11001/src/cmd/gc/fmt.c
File src/cmd/gc/fmt.c (right):

https://codereview.appspot.com/6866047/diff/11001/src/cmd/gc/fmt.c#newcode541
src/cmd/gc/fmt.c:541: return fmtprint(fp, "@\"\".%s", p);
Why is this right for s->pkg == builtinpkg? I would have expected that to refer
to the builtinpkg name we'd print plain %s not @"".%s.

https://codereview.appspot.com/6866047/diff/11001/test/fixedbugs/issue3552.go
File test/fixedbugs/issue3552.go (right):

https://codereview.appspot.com/6866047/diff/11001/test/fixedbugs/issue3552.go...
test/fixedbugs/issue3552.go:3: // Copyright 2012 The Go Authors. All rights
reserved.
you don't need to update copyright years

[remyoudompheng, 2012-12-22 10:59:43]

Hello lvd@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[DMorsing, 2012-12-22 12:58:18]

LGTM.

[rsc, 2012-12-22 14:31:04]

LGTM

[remyoudompheng, 2012-12-22 18:18:14]

*** Submitted as https://code.google.com/p/go/source/detail?r=6a028d6b7222 ***

cmd/gc: fix wrong interaction between inlining and embedded builtins.

The patch makes the compile user an ordinary package-local
symbol for the name of embedded fields of builtin type.

This is incompatible with the fix delivered for issue 2687
(revision 3c060add43fb) but fixes it in a different way, because
the explicit symbol on the field makes the typechecker able to
find it in lookdot.

Fixes issue 3552.

R=lvd, rsc, daniel.morsing
CC=golang-dev
https://codereview.appspot.com/6866047