Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/gc: problem with inlining + unexported field names #3552

Closed
gopherbot opened this issue Apr 19, 2012 · 12 comments
Closed

cmd/gc: problem with inlining + unexported field names #3552

gopherbot opened this issue Apr 19, 2012 · 12 comments
Milestone

Comments

@gopherbot
Copy link

by mt4swm:

What steps will reproduce the problem?

  go get hg.wmipf.de/issue.hg

then
  go build hg.wmipf.de/issue.hg/cmd

This will fail with the message:

 [...]issue.hg/cmd/main.go:4: issue.t.string undefined (cannot refer to unexported field or method issue.string)

In contrast, compiling the cmd package without inlining:
  go clean -i hg.wmipf.de/issue.hg
  go install -gcflags -l hg.wmipf.de/issue.hg
  go build -gcflags -l hg.wmipf.de/issue.hg/cmd
proceeds.

What is the expected output?

Package issue (http://hg.wmipf.de/issue/file/tip/pkg.go)
defines a struct type Token that also contains an anonymous
field of a base type, in this case: string. And it
defines a Method Token.String, that returns this anonymous
field.

Command package issue/cmd (http://hg.wmipf.de/issue/file/tip/cmd/main.go)
defines a
    `type t *issue.Token`

I would expect that "go build" in the cmd
directory succeeds, because, although Token contains
an unexported field "string", it isn't referenced from
outside the package.

What do you see instead?

Depending on whether inlining is active (default)
or inactive (-gcflags -l) compiling fails, or succeeds.

Which compiler are you using (5g, 6g, 8g, gccgo)?

8g

Which operating system are you using?

Debian Linux

Which version are you using?  (run 'go version')

weekly.2012-03-27 +362b760ecfc7

The behaviour can be observed since revision f7d0e6a40f06,
"gc: enable inlining by default".

Please provide any additional information below.

It seems that when package "issue" is imported, and
inlining is active, method Token.String is
somehow prepared for inlining; but the information,
that its access to the anonymous field "string" is valid,
because it occures in the same source package, gets
lost.

The problem seems to exist only with embedded base types.
If the anonymous field "string" is replaced with a differently
named type, witch has an underlying type "string", there
is no problem:

  type s  string

  type Token struct {
    name string
    s
  }

  func (t *Token) String() string {
    return string(t.s)
  }
@gopherbot
Copy link
Author

Comment 1 by mt4swm:

With the term "base types" I actually meant the predeclared boolean, numeric
and string type names.

@rsc
Copy link
Contributor

rsc commented Apr 20, 2012

Comment 2:

+lvd.  I haven't looked at this yet.

Status changed to Accepted.

@ianlancetaylor
Copy link
Contributor

Comment 3:

Labels changed: added priority-asap, compilerbug, removed priority-triage.

@lvdlvd
Copy link

lvdlvd commented Apr 23, 2012

Comment 4:

I'll look at it today.

@lvdlvd
Copy link

lvdlvd commented Apr 23, 2012

Comment 5:

Status changed to Started.

@lvdlvd
Copy link

lvdlvd commented Apr 23, 2012

Comment 6:

mt4swm's diagnosis was exactly right.
It looks like the fix for issue #2687 may have been a bit too pragmatic.  this unittest
reproduces the issue:
// one.go
package one
// issue #3552
type T struct { int }
func (t T) F() int { return t.int }
type U struct { int int }
func (u U) F() int { return u.int }
type lint int
type V struct { lint }
func (v V) F() int { return int(v.lint) }
type W struct { lint lint }
func (w W) F() int { return int(w.lint) }
// two.go
package two
import "./one"
func use() {
    var t one.T
    var u one.U
    var v one.V
    var w one.W
    _ = t.F()
    _ = u.F()
    _ = v.F()
    _ = w.F()
}
fixedbugs/bug434.dir/two.go:10: one.t.int undefined (cannot refer to unexported field or
method one.int)
(the others work fine, i just put them in for contrast)
the export section in one.6 reads:
$$  // exports
    package one
        import runtime "runtime"
        type @"".T struct { ? int }
        func (@"".t @"".T) F() (? int) { return @"".t.@"".int }
        type @"".U struct { @"".int int }
        func (@"".u @"".U) F() (? int) { return @"".u.@"".int }
        type @"".lint int
        type @"".V struct { ? @"".lint }
        func (@"".v @"".V) F() (? int) { return int(@"".v.@"".lint) }
        type @"".W struct { @"".lint @"".lint }
        func (@"".w @"".W) F() (? int) { return int(@"".w.@"".lint) }
putting a debug statement in typecheck lookdot1 shows it's looking up
lookdot1(@"/Users/lvd/Project/go3/test/one".int)   and finding @"".int -> no match.
An effective workaround is to not embed but name with the same name, eg.
type T {
    string string
}
since the builtin types have no methods, i think this is indistinguishable from the
original (except for this bug).

@lvdlvd
Copy link

lvdlvd commented Apr 23, 2012

Comment 7:

Labels changed: added priority-soon, removed priority-asap.

@lvdlvd
Copy link

lvdlvd commented Apr 23, 2012

Comment 8:

Owner changed to @lvdlvd.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 9:

Labels changed: added go1.1.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 10:

Labels changed: added size-l.

@remyoudompheng
Copy link
Contributor

Comment 11:

See issue #2687 for a related problem.
https://golang.org/cl/6866047/

@remyoudompheng
Copy link
Contributor

Comment 12:

This issue was closed by revision 9aef20e.

Status changed to Fixed.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants