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: backward incompatible change in Go 1.21 type inference with channels #62157

Closed
mokiat opened this issue Aug 19, 2023 · 10 comments
Closed
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. TypeInference Issue is related to generic type inference
Milestone

Comments

@mokiat
Copy link

mokiat commented Aug 19, 2023

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

$ go version
go version go1.21.0 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/momchil/Library/Caches/go-build'
GOENV='/Users/momchil/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/momchil/.go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/momchil/.go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/Cellar/go/1.21.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/Cellar/go/1.21.0/libexec/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.21.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/momchil/Workspace/Tools/gotest/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/w4/dj2vrtkn7x57h4fc45382jyw0000gn/T/go-build4050054595=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I tried to compile my project (that used to compile successfully with Go 1.20) but I get an error.

The project (with commit ref) is the following:
https://github.com/mokiat/gotest

The following command should run:

go test ./match

NOTE: While the project is just a playground at this point in time to see if generic matchers for testing and mocking might be possible in Go and the tests don't pass at the moment, the project does compile with Go 1.20 and does not with Go 1.21.

What did you expect to see?

ok      github.com/mokiat/gotest/match  1.317s

What did you see instead?

# github.com/mokiat/gotest/match_test [github.com/mokiat/gotest/match.test]
match/eventually_test.go:46:16: type match.Matcher[<-chan string] of Eventually(Produces(Eq("hello"))) does not match inferred type match.Matcher[chan string] for match.Matcher[T]
FAIL    github.com/mokiat/gotest/match [build failed]
FAIL

This can be fixed by casting the channel to a read-only channel.

Assert(c, ch, Eventually(Produces(Eq("hello"))))

needs to become

Assert(c, (<-chan string)(ch), Eventually(Produces(Eq("hello"))))

Nevertheless, this feels like a breaking change.

@dmitshur
Copy link
Contributor

Thanks for reporting. Can you check if this is the same issue as #61903, which was recently resolved at tip, or not the same issue?

@dmitshur dmitshur changed the title cmd/go: Backward incompatible change in Go 1.21 type inference with channels cmd/compile: backward incompatible change in Go 1.21 type inference with channels Aug 19, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 19, 2023
@mokiat
Copy link
Author

mokiat commented Aug 19, 2023

Hi,

I just tried with gotip and I still get an error though the formatting is slightly different (better):

$ gotip test ./match
# github.com/mokiat/gotest/match_test [github.com/mokiat/gotest/match.test]
match/eventually_test.go:46:16: cannot use Eventually(Produces(Eq("hello"))) (value of type match.Matcher[<-chan string]) as match.Matcher[chan string] value in argument to Assert: match.Matcher[<-chan string] does not implement match.Matcher[chan string] (wrong type for method Match)
                have Match(<-chan string) error
                want Match(chan string) error
FAIL    github.com/mokiat/gotest/match [build failed]
FAIL
$ gotip version
go version devel go1.22-a9859a7 Fri Aug 18 23:45:44 2023 +0000 darwin/amd64

To be honest, I am not sure which behavior is correct. In general Go does implicit casting from chan to <-chan or to chan<- and version Go 1.20 appeared to do the same with generic functions as well. Go 1.21 appears more strict and does not allow that now.

@mokiat
Copy link
Author

mokiat commented Aug 19, 2023

I have managed to simplify it to the following program:

package main

type Matcher[T any] func(T) bool

func Produces[T comparable](expected T) Matcher[<-chan T] {
	return func(ch <-chan T) bool {
		value := <-ch
		return value == expected
	}
}

func Assert[T any](a T, b Matcher[T]) {
	if !b(a) {
		panic("assertion failed!")
	}
}

func main() {
	ch := make(chan string, 1)
	ch <- "hello"
	Assert(ch, Produces("hello"))
}

This runs with Go 1.20 and does not compile with gotip or Go 1.21.

@dmitshur
Copy link
Contributor

CC @golang/compiler.

@dmitshur dmitshur added this to the Backlog milestone Aug 19, 2023
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 19, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.22 Aug 19, 2023
@ianlancetaylor
Copy link
Contributor

This is a compiler bug somewhere, because there is an order dependency in type inference. This program, which swaps the order of the arguments to Assert, does compile:

package main

type Matcher[T any] func(T) bool

func Produces[T comparable](expected T) Matcher[<-chan T] {
	return func(ch <-chan T) bool {
		value := <-ch
		return value == expected
	}
}

func Assert[T any](b Matcher[T], a T) {
	if !b(a) {
		panic("assertion failed!")
	}
}

func main() {
	ch := make(chan string, 1)
	ch <- "hello"
	Assert(Produces("hello"), ch)
}

Just swapping the argument order should not affect type inference, but for channel direction it apparently does.

CC @griesemer

@ianlancetaylor ianlancetaylor added the TypeInference Issue is related to generic type inference label Aug 19, 2023
@griesemer griesemer self-assigned this Aug 21, 2023
@griesemer
Copy link
Contributor

griesemer commented Aug 21, 2023

Simpler reproducer:

package p

func f[T any](T, T) {}

func _() {
	var a chan string
	var b <-chan string
	f(a, b)   // ERROR cannot use b (variable of type <-chan string) as chan string value in argument to f
	f(b, a)   // but this is ok
}

Definitely an inference/unification bug.

[Edit] This issue appears with both 1.20 and 1.21 so is a "pre-existing" error and slightly different from what was reported earlier. In any case, both of these should work.

@griesemer
Copy link
Contributor

The following simplified reproducer works with Go 1.20, but not with Go 1.21.

package p

type F[T any] func(T) bool

func g[T any](T) F[<-chan T] { return nil }

func f1[T any](T, F[T]) {}
func f2[T any](F[T], T) {}

func _() {
	var ch chan string
	f1(ch, g(""))
	f2(g(""), ch)
}

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/521500 mentions this issue: go/types, types2: remove order dependency in inference involving channels

@griesemer
Copy link
Contributor

@gopherbot please consider this for backport to 1.21, this is partly a regression and partly a pre-existing bug.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #62205 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. TypeInference Issue is related to generic type inference
Projects
None yet
Development

No branches or pull requests

5 participants