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: Identical ignores receiver of Signature #21367

Closed
dominikh opened this issue Aug 9, 2017 · 8 comments
Closed

go/types: Identical ignores receiver of Signature #21367

dominikh opened this issue Aug 9, 2017 · 8 comments

Comments

@dominikh
Copy link
Member

@dominikh dominikh commented Aug 9, 2017

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

go version go1.8.3 linux/amd64
go version devel +827be89a69 Thu Jun 15 21:53:23 2017 +0000 linux/amd64

What did you do?

In the go/types type system, methods are represented as Func objects
with Signature types, where the Signature has a receiver that is not
nil. To me, that implies that the receiver is part of the Signature's
identity. types.Identical, however, disagrees, and only looks at the
arguments and results. This leads to two methods, with differently
typed receivers, to be considered identical.

I can see arguments for and against this behaviour. On the one hand,
it makes correct canonicalisation difficult and leads to confusing
behaviour when using e.g. typeutil.Map. Two methods with different
receivers are clearly different, requiring a different implicit first
argument. Furthermore, swapping supposedly "identical" signatures
would change the meaning of the program.

On the other hand, when working with interfaces and checking
whether a method matches the definition in an interface, the receiver
is irrelevant.

I ran into this behavior while serialising type information to disk,
using typeutil.Map to deduplicate type information. This led to a
function turning into a method, because it shared the signature (sans
receiver) of a method.

A program demonstrating the issue is at https://play.golang.org/p/H0FR2qbm2k

I'm not sure if this is a bug or working as intended, but either way
it is a flaw in the current API.

/cc @griesemer

What did you expect to see?

func (pkg.T1).Method()
func (pkg.T2).Method()
false

What did you see instead?

func (pkg.T1).Method()
func (pkg.T2).Method()
true
@mvdan
Copy link
Member

@mvdan mvdan commented Aug 9, 2017

I would expect this function to take receivers into account too. However, if done that way, we would need to add an IdenticalIgnoringReceiver for the current uses.

On the other hand, if you don't want to ignore receivers, at the moment you could do Identical(T1, T2) && T1.Recv() == T2.Recv(). So I would imagine that clarifying this in the godoc is the right fix.

Loading

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

@griesemer griesemer commented Aug 9, 2017

I think this came up before (cc: @alandonovan ). We may not be able to do much more than documenting the existing behavior. But, depending on existing use, perhaps we can change the implementation. I don't recall at the moment why I decided to implement it the way it is (clearly there's a comment missing somewhere).

Loading

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Aug 9, 2017

I asked gri for the same change a long while back but we eventually convinced ourselves that the current behavior is correct. It is in any case too late to change the semantics, though we could improve the docs.

The gist of the argument is that the Go type system doesn't have a separate category for "method types"; the type of a method is a function type. Although we use a method's Signature as a convenient place to stash the type of its receiver, the type denoted by the Signature is identical to the specified type of the method, and there is no need to project away the receiver type. In this example,

type T int
func (T) f() {}

m := T(0).f

m has type func().

I'm sure it's possible to make it work using the alternative approach, and I tried at one point, but it didn't seem like a change worth making.

Loading

@alandonovan alandonovan closed this Aug 9, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Aug 9, 2017

Seems like we should keep the issue open to clarify the godoc of Identical.

Perhaps we should also clarify that the receiver is not important in a signature. Reading its godoc, it seems like it's as important as the parameters and results.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 9, 2017

@mvdan Agreed. Thanks for the backdrop, @alandonovan .

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 9, 2017

@alandonovan But in your example, T.f has type func(T). It depends how it's used. I think we could have gone either way as you say, but I'm pretty sure that I thought quite hard about the current choice as it impacted the go/types code (that knowledge is now implicit in the code). If I can dig up the reasoning, I'll add that to the documentation (besides documenting the current status).

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 15, 2017

Decided to simply document the status quo. The spec never talks about method signatures, only function signatures, and in go/types we compare function signatures.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 15, 2017

Change https://golang.org/cl/55710 mentions this issue: go/types: document that Signature.Recv() is ignored for type identity

Loading

@gopherbot gopherbot closed this in f6f125d Aug 16, 2017
@golang golang locked and limited conversation to collaborators Aug 16, 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
5 participants