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

spec: Method sets section doesn't seem quite right for interfaces with type lists #51183

Open
dominikh opened this issue Feb 14, 2022 · 13 comments
Assignees
Labels
Documentation NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dominikh
Copy link
Member

https://tip.golang.org/ref/spec#Method_sets says the following:

The method set of an interface type is the intersection of the method sets of each type in the interface's type set (the resulting method set is usually just the set of declared methods in the interface).

AFAIU, per

  • The type set of a non-empty interface is the intersection of the type sets of its interface elements.
  • The type set of a non-interface type term is the set consisting of just that type.

the type set of interface { S } for concrete type S is {S}. The intersection of the method sets is thus the method set of S. But that doesn't seem quite right. func foo[T S](x T) { x.Foo() } certainly doesn't compile. Was the intention to have an implicit, empty method set for the absent list of methods in the interface?

/cc @griesemer @findleyr

@dominikh dominikh added this to the Go1.18 milestone Feb 14, 2022
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 14, 2022
@ianlancetaylor
Copy link
Contributor

I'm not sure I completely understand the premise here. This code does work: https://go.dev/play/p/RFUCmHLu964?v=gotip . Sorry if I'm missing something obvious.

@dominikh
Copy link
Member Author

@ianlancetaylor https://go.dev/play/p/ZPFH8EReq1R?v=gotip is what I had in mind.

@ianlancetaylor
Copy link
Contributor

Ah, right. Thanks. Certainly the method set of S includes Foo(). I suppose the question here is: what is the method set of T and therefore of x? The current spec doesn't seem to specify the method set of a type parameter.

@ianlancetaylor
Copy link
Contributor

Or perhaps the call of x.Foo() is fine and this is a compiler bug (though perhaps not one that we should worry about for 1.18). After all, if all the types in a type set support a method, why not permit calling it?

@zigo101
Copy link

zigo101 commented Feb 15, 2022

Maybe, the description should be changed to

... is the combination of explicitly specified methods and the union of the method sets of each embedding interface type ...

(edited)

BTW, the following line has become invalid inaccurate now:

In a slightly more general form an interface T may use a (possibly qualified) interface type name E as an interface element. ...

Here the word name has become unnecessary in 1.18.

@zigo101
Copy link

zigo101 commented Feb 19, 2022

Should this be a 1.18 release blocker?

@griesemer griesemer self-assigned this Feb 19, 2022
@griesemer
Copy link
Contributor

@dominikh The intention (with type sets) has been to make this work, but we haven't gotten to it for 1.18. It's not a release blocker because there are trivial work-arounds: one can always wrap an interface with the necessary methods around the constraint (e.g. your example could be modified like this).

Given that we're very late in the release, we probably can't fix this in time (I'll have a look though, maybe it's trivial). We may end up documenting this as temporary restriction. Alternatively, maybe we can adjust the spec slightly and disallow this, but it needs some more thinking. Any less complexity is certainly appreciated.

@dominikh
Copy link
Member Author

I think it'd be simplest to leave the spec as is, and document the restriction. The behavior as described in the specification certainly makes sense and is consistent with the rest of the language.

@griesemer griesemer modified the milestones: Go1.18, Go1.19 Feb 24, 2022
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 24, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387924 mentions this issue: doc/go1.18: document method set limitation for method selectors

@griesemer griesemer added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Feb 24, 2022
gopherbot pushed a commit that referenced this issue Feb 25, 2022
For #51183.
For #47694.

Change-Id: If47ae074c3cd9f73b2e7f6408749d9a7d56bd8d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/387924
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@griesemer griesemer changed the title spec: Method sets section doesn't seem quite right for interfaces with type lists go/types, types2: Method sets section doesn't seem quite right for interfaces with type lists Feb 28, 2022
@gopherbot
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.19.
That time is now, so a friendly reminder to look at it again.

@gopherbot
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.20.
That time is now, so a friendly reminder to look at it again.

@griesemer griesemer removed the early-in-cycle A change that should be done early in the 3 month dev cycle. label Dec 14, 2022
@griesemer griesemer modified the milestones: Go1.20, Go1.21 Dec 14, 2022
@griesemer griesemer changed the title go/types, types2: Method sets section doesn't seem quite right for interfaces with type lists spec: Method sets section doesn't seem quite right for interfaces with type lists Jun 1, 2023
@griesemer griesemer modified the milestones: Go1.21, Go1.22 Jul 19, 2023
@griesemer
Copy link
Contributor

Moving this to 1.23. We hope to lift this and related restrictions eventually (see also #63940).

@griesemer griesemer modified the milestones: Go1.22, Go1.23 Dec 19, 2023
@griesemer
Copy link
Contributor

Didn't get to #63940, so moving this to 1.24 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants