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

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
Assignees
Labels
Milestone

Comments

@randall77
Copy link
Contributor

@randall77 randall77 commented Apr 28, 2021

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 this to the Go1.18 milestone Apr 28, 2021
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 28, 2021

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.

Loading

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

@danscales danscales commented Apr 28, 2021

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.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented May 7, 2021

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

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented May 12, 2021

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

Loading

gopherbot pushed a commit that referenced this issue May 12, 2021
… 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

@danscales danscales commented May 24, 2021

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.

Loading

@danscales danscales closed this May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants