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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: Incorrect error following named pointer dereference on field #11790

Closed
pto opened this issue Jul 19, 2015 · 7 comments
Closed

cmd/compile: Incorrect error following named pointer dereference on field #11790

pto opened this issue Jul 19, 2015 · 7 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@pto
Copy link

pto commented Jul 19, 2015

The fix to prevent incorrect dereference of a named pointer type on a method prevents an explicit dereference, but only when it follows a dereference of a field.

This relates to 40818cf and I am seeing the problem with go version go1.5beta2 darwin/amd64.

The following program (similar to the example in the Spec section on Selectors) gives the error q.M0 undefined. Strangely, this error only appears after first referencing q.x. If the q.x line is commented out, the following line succeeds. Reverse the order of the lines (put q.x after (*q).T0.M0()) and the program compiles and works!

package main

import "fmt"

type T0 struct {
    x int
}

func (*T0) M0() {
    fmt.Println("M0")
}

type T2 struct {
    *T0
}

type Q *T2

func main() {
    t0 := T0{42}
    t2 := T2{&t0}
    var q Q = &t2
    fmt.Println(q.x)  // Comment out either this line or the next line and the program works
    (*q).T0.M0()
}

What did you expect to see?

42
M0

What did you see instead?

./bug.go:24: q.M0 undefined (type Q has no field or method M0)
@adg adg changed the title Incorrect error following named pointer dereference on field cmd/compiler: Incorrect error following named pointer dereference on field Jul 20, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jul 20, 2015
@ianlancetaylor
Copy link
Contributor

Marking as 1.5 because this is a failure to compile a valid program.

CC @rsc

@ianlancetaylor ianlancetaylor changed the title cmd/compiler: Incorrect error following named pointer dereference on field cmd/compile: Incorrect error following named pointer dereference on field Jul 21, 2015
@rsc
Copy link
Contributor

rsc commented Jul 21, 2015

This was introduced as part of the fix to #9017 by @paranoiacblack. @dr2chase is looking into it.

@dr2chase
Copy link
Contributor

The root cause is the original sin of an AST that isn't really a tree. Because "q" is shared by different expressions, when it is marked "implicit" in one, it becomes "implicit" in another. I added tracing at the obvious places in the patch for #9017 ( b59dd94 ).

Note the mark-implicit on line 23 (q) and the error on 24 (because q is now implicit)

drchase-macbookair:tmp drchase$ go run b11790.go
# command-line-arguments
Marking implicit, line 23, node q (type Q)
ll = ll.Left, line 24, ll = (*q).T0 (type *T0)
ll = ll.Left, line 24, ll = *q (type T2)
./b11790.go:24: q.M0 undefined (type Q has no field or method M0)

But both actions are necessary for the 9017 test to pass (at line 53, specifically):

go build issue9017.go 
# command-line-arguments
ll = ll.Left, line 29, ll = s.T (type T)
ll = ll.Left, line 30, ll = s.T (type T)
ll = ll.Left, line 38, ll = ps (type S)
ll = ll.Left, line 39, ll = ps.T (type T)
Marking implicit, line 40, node ps (type *S)
ll = ll.Left, line 40, ll = ps.T (type T)
ll = ll.Left, line 46, ll = *p (type S)
ll = ll.Left, line 52, ll = p.T (type T)
Marking implicit, line 53, node p (type P)
ll = ll.Left, line 53, ll = p.T (type T)
./issue9017.go:47: p.mS undefined (type P has no field or method mS)
./issue9017.go:50: cannot use p (type P) as type I in assignment:
P does not implement I (missing mT method)
./issue9017.go:53: p.mT undefined (type P has no field or method mT)
./issue9017.go:56: cannot use p (type P) as type I in assignment:
P does not implement I (missing mT method)

So, I think we roll back the fix to 9017?

@gopherbot
Copy link

CL https://golang.org/cl/12532 mentions this issue.

@pto
Copy link
Author

pto commented Jul 24, 2015

With this change the compiler again allows the implicit dereference q.M0(). Was that the intention (reverting back to the Go 1.4.2 behavior), or should it reject q.M0() since (*q).M0 is not a field selector?

@dr2chase
Copy link
Contributor

Do you have a test case demonstrating the revert? I did not revert the change; I attempted to correct it, and the regression test for 9017 still passes.

@pto
Copy link
Author

pto commented Jul 24, 2015

My apologies. I think I must have gotten confused about which version of Go I was running. I recompiled from master and then I think I used the terminal that was still running 1.4. The test case I had is correctly reporting the q.M0 undefined error. Sorry, and thanks for the fix!

@golang golang locked and limited conversation to collaborators Aug 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
5 participants
@rsc @pto @dr2chase @ianlancetaylor @gopherbot