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

go/types: rejects valid program involving interfaces and cycles #21804

Closed
dominikh opened this issue Sep 8, 2017 · 10 comments
Closed

go/types: rejects valid program involving interfaces and cycles #21804

dominikh opened this issue Sep 8, 2017 · 10 comments
Assignees
Milestone

Comments

@dominikh
Copy link
Member

@dominikh dominikh commented Sep 8, 2017

What version of Go are you using (go version)?

go version devel +6b6b9f69fd Wed Aug 9 05:33:09 2017 +0000 linux/amd64
go version go1.9 linux/amd64
go version go1.8.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

https://play.golang.org/p/Vne0Zww-Qf

What did you expect to see?

package gps ("qps") <nil>

What did you see instead?

package gps ("qps") main.go:37:20: cannot use UnpairedVersion(nil) (value of type UnpairedVersion) as Constraint value in variable declaration: missing method copyTo

The Go compiler accepts this code. The example is based on a (much, much larger) real code base (golang/dep#1127)

/cc @griesemer

@jmank88
Copy link

@jmank88 jmank88 commented Sep 8, 2017

I'm unsure if this adds insight, but I was surprised that changing this signature from a method to a function matters:

-func (m *ConstraintMsg) asUnpairedVersion() UnpairedVersion {
+func asUnpairedVersion(m *ConstraintMsg) UnpairedVersion {
	return nil
}

Loading

@dominikh
Copy link
Member Author

@dominikh dominikh commented Oct 23, 2017

Ping. Is there anybody familiar enough with go/types to look at this? /cc @griesemer

Loading

@griesemer griesemer self-assigned this Oct 23, 2017
@griesemer griesemer added this to the Go1.10 milestone Oct 23, 2017
@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 23, 2017

@dominikh I will need to look into this. Is this a blocker for you? It seems that there's a work-around (moving the type to the bottom of the file) that doesn't affect the semantics of the code.

Loading

@jmank88
Copy link

@jmank88 jmank88 commented Oct 23, 2017

The original dep issue is no longer a problem. I have not encountered any other instances in the wild.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 23, 2017

Simplified reproducer:

package p

type _ interface{ m(B) }

type A interface{ a(D) }
type B interface{ A }
type C interface{ B }
type D interface{ C }

var _ A = C(nil)

gotype reports:

x.go:10:11: cannot use C(nil) (value of type C) as A value in variable declaration: missing method a

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 23, 2017

This is most likely a duplicate of #18395.

Loading

@dominikh
Copy link
Member Author

@dominikh dominikh commented Oct 24, 2017

It's not a blocker per se. Telling users of static analysis tools to move their code around is a subpar experience, but the one occurrence in dep was the first and only time I actually ran into the problem in the real world, so it's not a high priority from my end.

Might be a duplicate, yeah.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 24, 2017

@dominikh I'm not suggesting that moving the code around is a solution, it's a work-around. Knowing there is a work-around is useful for prioritizing an issue. Clearly we need to fix this, but there's a lot of other issues we need to fix as well...

Loading

@dominikh
Copy link
Member Author

@dominikh dominikh commented Oct 24, 2017

@griesemer Right. We're in agreement.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Nov 21, 2017

Confirmed duplicate of #18395 by running with https://go-review.googlesource.com/c/go/+/78955 in debug mode.

Loading

@griesemer griesemer closed this Nov 21, 2017
@golang golang locked and limited conversation to collaborators Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants