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, go/types, types2: restore Go 1.20 unification when compiling for Go 1.20 #61903

Closed
vagruchi opened this issue Aug 9, 2023 · 10 comments
Closed
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@vagruchi
Copy link

vagruchi commented Aug 9, 2023

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

1.21.0

Does this issue reproduce with the latest release?

yes

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

I can reproduce this issue using playground only

What did you do?

use code

// You can edit this code!
// Click here and start typing.
package main

import "fmt"

type GenericType[T any] interface{}

func get[T any](v GenericType[T]) {

}

func use() {
	var f GenericType[int]
	get(f)
}

func main() {}

What did you expect to see?

no error as it is with go 1.20 https://go.dev/play/p/CEVB_qbwsPu?v=goprev

What did you see instead?

error ./prog.go:13:5: cannot infer T (prog.go:7:10) https://go.dev/play/p/CEVB_qbwsPu

@seankhliao
Copy link
Member

@ianlancetaylor @griesemer

@findleyr
Copy link
Contributor

findleyr commented Aug 9, 2023

I believe this is a consequence of #60377.

@findleyr
Copy link
Contributor

findleyr commented Aug 9, 2023

This is definitely a consequence of #60377. At the time, this was considered bug in the type inference algorithm that was inconsistent with looking inside the interfaces (per #41176).

CC @rsc for awareness.

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 9, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 9, 2023
@rsc
Copy link
Contributor

rsc commented Aug 10, 2023

It sounds like we should at least restore the old behavior when go.mod says "go 1.20".
Let's try to do that in the next point release.

@rsc rsc changed the title breaking change: generics type inference with generic interface spec: restore Go 1.20 unification when compiling for Go 1.20 Aug 10, 2023
@rsc rsc modified the milestones: Backlog, Go1.21.1 Aug 10, 2023
@dmitshur
Copy link
Contributor

The fix needs to land on the main branch first (i.e., where Go 1.2​2 development happens), and then it can be backported to release-branch.go1.21. I'll move this to Go1.2​2 milestone and create a backport issue to track the fix for Go 1.21.1.

@gopherbot Please backport to Go 1.21 with the rationale @rsc provided:

[...] we should at least restore the old behavior when go.mod says "go 1.2​0".
Let's try to do that in the next point release.

@dmitshur dmitshur modified the milestones: Go1.21.1, Go1.22 Aug 10, 2023
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #61930 (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.

@findleyr findleyr changed the title spec: restore Go 1.20 unification when compiling for Go 1.20 spec, go/types, types2: restore Go 1.20 unification when compiling for Go 1.20 Aug 15, 2023
@griesemer griesemer self-assigned this Aug 15, 2023
@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 15, 2023
@griesemer
Copy link
Contributor

The 1.20 behavior was incorrect. For instance, taking the (slightly pared down, but equivalent) example from above:

package main

type T[P any] interface{}

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

func main() {
	var x T[int]
	f(x)
}

which works with Go1.20 but doesn't work with Go1.21 (because any arbitrary type argument would be acceptable for type parameter P and hence we can't infer a specific one), and now we add an extra parameter to f like so:

package main

type T[P any] interface{}

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

func main() {
	var x T[int]
	var y string
	f(x, y)
}

Now this code doesn't work with Go1.20 because the inferred argument for P is wrong, but it works with Go1.21.

We can try to restore the incorrect behavior for 1.20, and it will "fix" this particular case, but it will also re-introduce a bug.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/519855 mentions this issue: go/types, types2: disable interface inference for versions before Go 1.21

@rsc
Copy link
Contributor

rsc commented Aug 16, 2023

I wouldn't say that it introduces bugs so much as it preserves them, which is what compatibility tends to be all about. 😄
We've said from the start that if we had some kind of show-stopper bug in generics that needed fixing, the way we'd do that compatibly is to key the behavior off the Go language version. This is not a show-stopper but it came up in at least one person's real programs, so it's worth keeping the compatibility in old code.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/520195 mentions this issue: Revert "go/types, types2: remove internal constant enableInterfaceInference"

gopherbot pushed a commit that referenced this issue Aug 17, 2023
…erence"

This reverts CL 514715.

This will make it easier to make interface inference conditional
based on the current language version.

For #61903.

Change-Id: I07820c861d6ebfd04899e41eb4123f26af2da1ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/520195
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

8 participants