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: constraint type inference should proceed even if an individual unification step fails #53650
Comments
CC @griesemer Standalone reproducer: package p
type BaseT interface {
Type1 | Type2
}
type BaseType int
type Type1 BaseType
type Type2 BaseType
type ValueT[T BaseT] struct {
A1 T
}
func NewType1() *ValueT[Type1] {
r := NewT[Type1]()
return r
}
func NewT[TInt BaseT, TVal ValueT[TInt]]() *TVal {
ret := TVal{}
return &ret
}
|
I'm not good at generics, maybe there is a flaw here. And I find the compiler will be happy with these cases: Case one based on @RelicOfTesla's code: type BaseT interface {
Type1 | Type2
}
type BaseType int
type Type1 BaseType
type Type2 BaseType // float64
type ValueT[T BaseT] struct {
A1 T
}
func NewType1() *ValueT[Type1] {
r := NewT[Type1, ValueT[Type1]]()
return r
}
func NewType2() *ValueT[Type2] {
r := NewT[Type2, ValueT[Type2]]() // add `, ValueT[Type2]`
return r
}
func NewT[TBase BaseT, TVal ValueT[TBase]]() *TVal {
ret := TVal{}
return &ret
}
func TestGoType(t *testing.T) {
r1 := NewType1()
r2 := NewType2()
t.Log(r1, r2)
t.Log(reflect.TypeOf(r1), reflect.TypeOf(r2))
fooT1(r1.A1)
fooT2(r2.A1)
}
func fooT1(t1 Type1) {
}
func fooT2(t2 Type2) {
} Case two based on @ianlancetaylor's code: package p
type BaseT interface {
Type1 | Type2
}
type BaseType int
type Type1 BaseType
type Type2 BaseType
type ValueT[T BaseT] struct {
A1 T
}
func NewType1() *ValueT[Type1] {
r := NewT[Type1, ValueT[Type1]]() // add `, ValueT[Type1]`
return r
}
func NewT[TInt BaseT, TVal ValueT[TInt]]() *TVal {
ret := TVal{}
return &ret
} Case three is modified by me: package p
type BaseT interface {
Type1 | Type2
}
type Type1 int
type Type2 int
var _ = NewT[Type1, Type1]() // add `Type1` or `Type2`
func NewT[TInt, TVal BaseT]() TVal {
return TVal(0)
} From the results, it seems that we do not support indirect type deduction |
Smaller reproducer: type T1 int
type T2 int // code works if this is changed to, say int8
type G[_ T1 | T2] struct{}
func _() {
f[T1]()
f[T2]()
}
func f[P T1 | T2, Q G[P]]() {} This works: type T1 struct{}
type T2 struct{}
type G[_ T1 | T2] struct{}
func _() {
_ = f[T1]
}
func f[P T1 | T2, Q G[P]]() {} |
Even smaller reproducer: type T1 int
type T2 int
func f[P T1 | T2, _ []P]() {}
var _ = f[T1] This is a bug with constraint type inference: If If So constraint type inference is a bit too aggressive here. This area is notoriously tricky. May not be safe to address at this point in the release cycle (TBD). Thinking. For the release team: It's safe to kick this down to Go 1.21 if we don't find a safe fix in time. This is not a regression; the bug existed since 1.18. Also, providing all type arguments manually is a safe work-around. FYI @findleyr |
Change https://go.dev/cl/454655 mentions this issue: |
Per discussion with @findleyr, we believe https://go.dev/cl/454655 is almost certainly correct, but there's no rush to fix this for 1.20. Still thinking about possible ramifications. |
In the section on constraint type inference, the spec states:
which is exactly what the implementation is doing. The fix (https://go.dev/cl/454655) does something else:
I believe this new approach is the correct approach (and does fix this issue), but it is in fact a (backward-compatible, I think) spec change. In other words, the current behavior is "correct" per the spec. Making this a spec change and moving it to 1.21. |
Change https://go.dev/cl/470916 mentions this issue: |
The primary change is that type inference now always reports an error if a unification step fails (rather than ignoring that case, see infer2.go). This brings the implementation closely to the description in #58650; but the implementation is more direct by always maintaining a simple (type parameter => type) mapping. To make this work, there are two small but subtle changes in the unifier: 1) When deciding whether to proceed with the underlying type of a defined type, we also use the underlying type if the other type is a basic type (switch from !hasName(x) to isTypeLit(x) in unifier.go). This makes the case in issue #53650 work out. See the comment in the code for a detailed explanation of this change. 2) When we unify against an unbound type parameter, we always proceed with its core type (if any). Again, see the comment in the code for a detailed explanation of this change. The remaining changes are comment and test adjustments. Because the new logic now results in failing type inference where it succeeded before or vice versa, and then instatiation or parameter passing failed, a handful of error messages changed. As desired, we still have the same number of errors for the same programs. Also, because type inference now produces different results, we cannot easily compare against infer1 anymore (also infer1 won't work correctly anymore due to the changes in the unifier). This comparison (together with infer1) is now disabled. Because some errors and their positions have changed, we need a slightly larger error position tolerance for types2 (which produces less accurate error positions than go/types). Hence the change in types2/check_test.go. Finally, because type inference is now slightly more relaxed, issue #51139 doesn't produce a type unification failure anymore for a (previously correctly) inferred type argument. Fixes #51139. Change-Id: Id796eea42f1b706a248843ad855d9d429d077bd1 Reviewed-on: https://go-review.googlesource.com/c/go/+/470916 Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com>
Edit: See #53650 (comment) for a summary of the problem.
What version of Go are you using (
go version
)?What did you do?
What did you expect to see?
Compile success
What did you see instead?
error:
The text was updated successfully, but these errors were encountered: