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: generics with method values not working #45817

Closed
randall77 opened this issue Apr 28, 2021 · 5 comments
Closed

cmd/compile: generics with method values not working #45817

randall77 opened this issue Apr 28, 2021 · 5 comments
Labels
FrozenDueToAge generics Issue is related to generics
Milestone
Go1.18

Comments

@randall77
Copy link
Contributor 

package main

type s[T any] struct {
	a T
}
func (x s[T]) f() T {
	return x.a
}
func main() {
	x := s[int]{a:7}
	f := x.f
	f()
}

% ../bin/go run -gcflags=-G=3 ~/gowork/tmp6.go
# command-line-arguments
../../../gowork/tmp6.go:11:4: cannot use s[int].f (type func(s[int]) int) as type func() int in assignment

This should work, I think. It looks like it got the type of f correct, which means := worked. But then later it decided the RHS was a different type...

I think this might be happening during stenciling. Beforehand, the RHS of the assignment is

.   .   .   FUNCINST tc(1) FUNC-func() int # tmp6.go:11 FUNC-func() int
.   .   .   .   CALLPART tc(1) FUNC-method(s[T₂]) func() T₂ # tmp6.go:11 main.s[T₂].f FUNC-method(s[T₂]) func() T₂
.   .   .   .   .   NAME-main.x tc(1) Class:PAUTO Offset:0 OnStack main.s[int] # tmp6.go:10

afterward, the RHS is

.   .   .   NAME-main.s[int].f tc(1) Class:PFUNC Offset:0 FUNC-func(s[int]) int # tmp6.go:6

The former looks superficially ok, but I'm not entirely sure as I don't really understand what method types are.
The latter definitely looks like a problem.

Adding -l to the command line makes the compiler crash :(

@griesemer @findleyr @mdempsky @danscales
@randall77 randall77 added the generics Issue is related to generics label Apr 28, 2021
@randall77 randall77 added this to the Go1.18 milestone Apr 28, 2021
@mdempsky
Copy link
Contributor

Method types are just function types with the receiver parameter still in place. As far as expression typing goes, you can generally just ignore the receiver part: method(A) func(B) C is identical to func(B) C within the Go type system.

Off hand, I think they only show up in ODOTMETH, ODOTINTER, and OCALLPART. But even there, we could probably change them to use the bare function signatures if desirable. (We used to rely on these nodes referencing the exact *types.Type of the function, so we could get back to the corresponding *ir.Name/*ir.Func. But that's no longer necessary since we added ir.SelectorExpr.Selection.)

Method expressions will instead have a new type like func(B, C) anyway.

@danscales danscales self-assigned this Apr 28, 2021
@danscales
Copy link
Contributor

Thanks for the example. Method values are definitely an untested area. I think I already have another example of method values not working when I tried an adjusted version of ./cmd/compile/internal/types2/fixedbugs/issue44688.go2.

FYI, the other big areas that are not handled correctly yet related to generic functions/types are: embedded fields & interfaces and type aliases.

I eventually plan to go through all of ./cmd/compile/internal/types2/fixedbugs, but it will be easier once we have basic export/import working.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/318089 mentions this issue: cmd/compile: fix use of method values with stenciled methods

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/319589 mentions this issue: cmd/compile: fix use of method values with stenciled methods

gopherbot pushed a commit that referenced this issue May 12, 2021
@danscales
[dev.typeparams] cmd/compile: fix use of method values with stenciled…
04f65d3 
… methods

We were handling the case where an OFUNCINST node was used as a function
value, but not the case when an OFUNCINST node was used as a method
value. In the case of a method value, we need to create a new selector
expression that references the newly stenciled method.

To make this work, also needed small fix to noder2 code to properly set the
Sel of a method SelectorExpr (should be just the base method name, not
the full method name including the type string). This has to be correct,
so that the function created by MethodValueWrapper() can be typechecked
successfully.

Fixes #45817

Change-Id: I7343e8a0d35fc46b44dfe4d45b77997ba6c8733e
Reviewed-on: https://go-review.googlesource.com/c/go/+/319589
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
@danscales
Copy link
Contributor

Fixed by https://golang.org/cl/319589. I guess the issue wasn't closed automatically, because the CL was checked in on dev.typeparams? But I think we can close this issue, since we're doing all the testing/dev on dev.typeparams.

@golang golang locked and limited conversation to collaborators May 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge generics Issue is related to generics
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@mdempsky @danscales @randall77 @gopherbot