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: type inference appears to infer underlying types #50426

Closed
ivokub opened this issue Jan 4, 2022 · 4 comments
Closed

cmd/compile: type inference appears to infer underlying types #50426

ivokub opened this issue Jan 4, 2022 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@ivokub
Copy link

ivokub commented Jan 4, 2022

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

$ go version 
go1.18beta1 linux/amd64

Does this issue reproduce with the latest release?

With Go 1.18 beta 1

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ivo/.cache/go-build"
GOENV="/home/ivo/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ivo/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ivo/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ivo/sdk/go1.18beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ivo/sdk/go1.18beta1/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18beta1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ivo/Work/workspace/gnark/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1880804062=/tmp/go-build -gno-record-gcc-switches"


The situation is reproducible with Go Playground on develop.

What did you do?

Trying out union interface types. The following program does not compile:

type A1 [2]uint64
type A2 [2]uint64
func (a A1) Get() A1 {	return a}
func (a A2) Get() A2 {	return a}
type I[T any] interface {
	A1 | A2
	Get() T
}
func F[B any, T I[B]](v T) {	v.Get()}
func main() {
	v := A2{1, 2}
	F[A2](v)
}

Go Play link

However, when I add another type A3 [3]uint64, then compiles and runs:

type A1 [2]uint64
type A2 [2]uint64
type A3 [3]uint64
func (a A1) Get() A1 {	return a}
func (a A2) Get() A2 {	return a}
func (a A3) Get() A3 {  return a}
type I[T any] interface {
	A1 | A2 | A3 // <-- added A3
	Get() T
}
func F[B any, T I[B]](v T) {	v.Get()}
func main() {
	v := A2{1, 2}
	F[A2](v)
}

Go Play link

What did you expect to see?

Nothing

What did you see instead?

./prog.go:19:7: [2]uint64 does not implement I[A2] (missing method Get)

@findleyr
Copy link
Contributor

findleyr commented Jan 4, 2022

CC @griesemer

@findleyr findleyr added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 4, 2022
@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Jan 4, 2022
@griesemer griesemer self-assigned this Jan 5, 2022
@griesemer
Copy link
Contributor

griesemer commented Jan 7, 2022

This appears to be an issue (possibly a couple of issues) with type inference. Here's a simpler reproducer:

package p

type A1 [2]uint64
type A2 [2]uint64

func (a A1) m() A1 { return a }
func (a A2) m() A2 { return a }

func f[B any, T interface{A1|A2; m() T}](v T) {}

func _() {
        var v A2
	f[A2](v)
}

Error: [2]uint64 does not implement interface{m() [2]uint64; A1|A2} (missing method m)

Providing an (arbitrary) type argument for B (as in f[int, A2]) or removing the type parameter B "fixes" this.

I also note that in the original example, there's a difference between A1, A2, and A3: A3 has a different length then the other two which seems to "make it work".

Also, providing all type arguments explicitly (F[A2, A2](v) instead of F[A2](v)) makes the first example work.

@griesemer griesemer changed the title cmd/compile: types are unified too eagerly in union type interface cmd/compile: type inference appears to infer underlying types Jan 7, 2022
@griesemer
Copy link
Contributor

The cause here is that - per the current algorithm for type inference - constraint type inference infers the wrong type: in the instantiation f[A2] we only have one type argument (A2 for the type parameter B - in fact this type could be anything because it's not used). Currently, constraint type inference takes precedence over function type inference (and this may be the underlying problem) and tries to infer the 2nd type argument. This is successful because the constraint interface{A1|A2; m() T} has a structural type which is [2]uint which is not generic. So type inference infers the type argument [2]uint for the 2nd type parameter, and this array type doesn't have a method m and thus the code fails.

In the original example, when adding the A3 array, the resulting constraint doesn't have a structural type anymore and constraint type inference is not successful. Instead inference continues with the function argument and then successfully infers the correct type, which is why that version works.

cc: @ianlancetaylor

@gopherbot
Copy link

Change https://golang.org/cl/377594 mentions this issue: go?types, types2: do not run CTI before FTI

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
Until now, CTI (constraint type inference) was run before
FTI (function type inference). This lead to situations
where CTI infered a type that is missing necessary methods
even though a function argument of correct type was given.
This can happen when constraint type inference produces a
inferred type that is the structural type of multiple types,
which then is an underlying type, possibly without methods.

This CL removes the initial CTI step; it is only applied
after FTI with type arguments is run, and again after FTI
with untyped arguments is run.

Various comments are adjusted to reflect the new reality.

Fixes golang#50426.

Change-Id: I700ae6e762d7aa00d742943a2880f1a1db33c2b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/377594
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants