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

go/types, types2: type parameter order is significant when it should not be #40789

Closed
rogpeppe opened this issue Aug 14, 2020 · 5 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Aug 14, 2020

commit 722af87

The following program fails with the error "K does not satisfy comparable", in the copyMap function definition when AFAICS K is declared correctly as having a comparable constraint.

package main

import "fmt"

func main() {
	m := map[string]int{
		"a": 6,
		"b": 7,
	}
	fmt.Println(copyMap[map[string]int, string, int](m))
}

type Map[K comparable, V any] interface {
	type map[K] V
}

func copyMap[M Map[K, V], K comparable, V any](m M) M {
	m1 := make(M)
	for k, v := range m {
		m1[k] = v
	}
	return m1
}```

If the type parameters are reordered to put `K` first, the example [works OK](https://go2goplay.golang.org/p/JwcN6NspdYu).
@rogpeppe rogpeppe changed the title cmd/go2go: cmd/go2go: type parameter order is significant when it should not be Aug 14, 2020
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 14, 2020
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Aug 14, 2020
@AndrewWPhillips
Copy link
Contributor

Here's something simpler to demonstrate the problem.

package main

type A[X comparable] interface {
	type []X
}

func f[B A[X], X comparable]() B {
	return nil
}

func main() {
}

@griesemer
Copy link
Contributor

Thanks for this. Another case where the prototype type-checks too eagerly. This is a problem we need to solve in general in a real implementation (probably requires a two-phase approach). Addressing this should resolve a large number of bugs (not just with generics but elsewhere as well).

@griesemer griesemer modified the milestones: Unreleased, Go1.17 Feb 17, 2021
@griesemer griesemer modified the milestones: Go1.17, Go1.18 Apr 14, 2021
@griesemer griesemer changed the title cmd/go2go: type parameter order is significant when it should not be go/types, types2: type parameter order is significant when it should not be Jun 29, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/331690 mentions this issue: [dev.typeparams] cmd/compile/internal/types2: delay interface check for type bounds

@griesemer
Copy link
Contributor

The above CL fixes this issue for types2. The initial example still doesn't fully compile due to an unrelated error in the compiler.

@griesemer griesemer added release-blocker 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 Jun 29, 2021
gopherbot pushed a commit that referenced this issue Jul 1, 2021
…or type bounds

While at it, clean up code for collecting/declaring type parameters.

For #40789.

Change-Id: I0855137d5ee85c0ae2fa60d33b28c24a33132fbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/331690
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@griesemer
Copy link
Contributor

This was fixed in the type checkers by

https://golang.org/cl/331690 in types2
https://golang.org/cl/335034 in go/types

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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

5 participants