diff --git a/src/cmd/compile/internal/types2/infer.go b/src/cmd/compile/internal/types2/infer.go index df87f8da4fb..6259e287aed 100644 --- a/src/cmd/compile/internal/types2/infer.go +++ b/src/cmd/compile/internal/types2/infer.go @@ -42,7 +42,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, } if traceInference { - check.dump("-- inferA %s ➞ %s", tparams, targs) + check.dump("-- inferA %s%s ➞ %s", tparams, params, targs) defer func() { check.dump("=> inferA %s ➞ %s", tparams, result) }() @@ -61,6 +61,74 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type, } // len(targs) < n + const enableTparamRenaming = true + if enableTparamRenaming { + // For the purpose of type inference we must differentiate type parameters + // occurring in explicit type or value function arguments from the type + // parameters we are solving for via unification, because they may be the + // same in self-recursive calls. For example: + // + // func f[P *Q, Q any](p P, q Q) { + // f(p) + // } + // + // In this example, the fact that the P used in the instantation f[P] has + // the same pointer identity as the P we are trying to solve for via + // unification is coincidental: there is nothing special about recursive + // calls that should cause them to conflate the identity of type arguments + // with type parameters. To put it another way: any such self-recursive + // call is equivalent to a mutually recursive call, which does not run into + // any problems of type parameter identity. For example, the following code + // is equivalent to the code above. + // + // func f[P interface{*Q}, Q any](p P, q Q) { + // f2(p) + // } + // + // func f2[P interface{*Q}, Q any](p P, q Q) { + // f(p) + // } + // + // We can turn the first example into the second example by renaming type + // parameters in the original signature to give them a new identity. As an + // optimization, we do this only for self-recursive calls. + + // We can detect if we are in a self-recursive call by comparing the + // identity of the first type parameter in the current function with the + // first type parameter in tparams. This works because type parameters are + // unique to their type parameter list. + selfRecursive := check.sig != nil && check.sig.tparams.Len() > 0 && tparams[0] == check.sig.tparams.At(0) + + if selfRecursive { + // In self-recursive inference, rename the type parameters with new type + // parameters that are the same but for their pointer identity. + tparams2 := make([]*TypeParam, len(tparams)) + for i, tparam := range tparams { + tname := NewTypeName(tparam.Obj().Pos(), tparam.Obj().Pkg(), tparam.Obj().Name(), nil) + tparams2[i] = NewTypeParam(tname, nil) + tparams2[i].index = tparam.index // == i + } + + renameMap := makeRenameMap(tparams, tparams2) + for i, tparam := range tparams { + tparams2[i].bound = check.subst(pos, tparam.bound, renameMap, nil) + } + + tparams = tparams2 + params = check.subst(pos, params, renameMap, nil).(*Tuple) + + // If we replaced any type parameters, their replacements may occur in + // the resulting inferred type arguments. Make sure we use the original + // type parameters in the result. + defer func() { + unrenameMap := makeRenameMap(tparams2, tparams) + for i, res := range result { + result[i] = check.subst(pos, res, unrenameMap, nil) + } + }() + } + } + // If we have more than 2 arguments, we may have arguments with named and unnamed types. // If that is the case, permutate params and args such that the arguments with named // types are first in the list. This doesn't affect type inference if all types are taken diff --git a/src/cmd/compile/internal/types2/subst.go b/src/cmd/compile/internal/types2/subst.go index f2e8fecc05b..44a59f55fd8 100644 --- a/src/cmd/compile/internal/types2/subst.go +++ b/src/cmd/compile/internal/types2/subst.go @@ -21,6 +21,17 @@ func makeSubstMap(tpars []*TypeParam, targs []Type) substMap { return proj } +// makeRenameMap is like makeSubstMap, but creates a map used to rename type +// parameters in from with the type parameters in to. +func makeRenameMap(from, to []*TypeParam) substMap { + assert(len(from) == len(to)) + proj := make(substMap, len(from)) + for i, tpar := range from { + proj[tpar] = to[i] + } + return proj +} + func (m substMap) empty() bool { return len(m) == 0 } diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48619.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48619.go2 index 3d4f1b4707f..72eea1ef596 100644 --- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48619.go2 +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48619.go2 @@ -2,24 +2,19 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This issue is still open: -// - the error messages could be better or are incorrect -// - unification fails due to stack overflow that is caught - package p func f[P any](a, _ P) { var x int // TODO(gri) these error messages, while correct, could be better - f(a, x /* ERROR type int of x does not match P */) + f(a, x /* ERROR type int of x does not match inferred type P for P */) f(x, a /* ERROR type P of a does not match inferred type int for P */) } func g[P any](a, b P) { g(a, b) - // TODO(gri) these error messages are incorrect because the code is valid - g(&a, & /* ERROR type \*P of &b does not match inferred type \*P for P */ b) - g([]P{}, [ /* ERROR type \[\]P of \[\]P{} does not match inferred type \[\]P for P */ ]P{}) + g(&a, &b) + g([]P{}, []P{}) // work-around: provide type argument explicitly g[*P](&a, &b) diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48656.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48656.go2 index bea3dc14a0e..0f60f47120a 100644 --- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48656.go2 +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48656.go2 @@ -2,14 +2,12 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This issue is still open: -// - the error messages are unclear -// - unification fails due to stack overflow that is caught - package p func f[P *Q, Q any](P, Q) { - // TODO(gri) these error messages are unclear - _ = f[ /* ERROR P does not match \*Q */ P] - _ = f[ /* ERROR cannot infer P */ *P] + _ = f[P] +} + +func f2[P /* ERROR instantiation cycle */ *Q, Q any](P, Q) { + _ = f2[*P] } diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue51158.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue51158.go2 new file mode 100644 index 00000000000..3edc5053820 --- /dev/null +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue51158.go2 @@ -0,0 +1,18 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +// Type checking the following code should not cause an infinite recursion. +func f[M map[K]int, K comparable](m M) { + f(m) +} + +// Equivalent code using mutual recursion. +func f1[M map[K]int, K comparable](m M) { + f2(m) +} +func f2[M map[K]int, K comparable](m M) { + f1(m) +} diff --git a/src/go/types/infer.go b/src/go/types/infer.go index b4b6b780169..18ec81edd43 100644 --- a/src/go/types/infer.go +++ b/src/go/types/infer.go @@ -41,7 +41,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, } if traceInference { - check.dump("-- inferA %s ➞ %s", tparams, targs) + check.dump("-- inferA %s%s ➞ %s", tparams, params, targs) defer func() { check.dump("=> inferA %s ➞ %s", tparams, result) }() @@ -60,6 +60,74 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type, } // len(targs) < n + const enableTparamRenaming = true + if enableTparamRenaming { + // For the purpose of type inference we must differentiate type parameters + // occurring in explicit type or value function arguments from the type + // parameters we are solving for via unification, because they may be the + // same in self-recursive calls. For example: + // + // func f[P *Q, Q any](p P, q Q) { + // f(p) + // } + // + // In this example, the fact that the P used in the instantation f[P] has + // the same pointer identity as the P we are trying to solve for via + // unification is coincidental: there is nothing special about recursive + // calls that should cause them to conflate the identity of type arguments + // with type parameters. To put it another way: any such self-recursive + // call is equivalent to a mutually recursive call, which does not run into + // any problems of type parameter identity. For example, the following code + // is equivalent to the code above. + // + // func f[P interface{*Q}, Q any](p P, q Q) { + // f2(p) + // } + // + // func f2[P interface{*Q}, Q any](p P, q Q) { + // f(p) + // } + // + // We can turn the first example into the second example by renaming type + // parameters in the original signature to give them a new identity. As an + // optimization, we do this only for self-recursive calls. + + // We can detect if we are in a self-recursive call by comparing the + // identity of the first type parameter in the current function with the + // first type parameter in tparams. This works because type parameters are + // unique to their type parameter list. + selfRecursive := check.sig != nil && check.sig.tparams.Len() > 0 && tparams[0] == check.sig.tparams.At(0) + + if selfRecursive { + // In self-recursive inference, rename the type parameters with new type + // parameters that are the same but for their pointer identity. + tparams2 := make([]*TypeParam, len(tparams)) + for i, tparam := range tparams { + tname := NewTypeName(tparam.Obj().Pos(), tparam.Obj().Pkg(), tparam.Obj().Name(), nil) + tparams2[i] = NewTypeParam(tname, nil) + tparams2[i].index = tparam.index // == i + } + + renameMap := makeRenameMap(tparams, tparams2) + for i, tparam := range tparams { + tparams2[i].bound = check.subst(posn.Pos(), tparam.bound, renameMap, nil) + } + + tparams = tparams2 + params = check.subst(posn.Pos(), params, renameMap, nil).(*Tuple) + + // If we replaced any type parameters, their replacements may occur in + // the resulting inferred type arguments. Make sure we use the original + // type parameters in the result. + defer func() { + unrenameMap := makeRenameMap(tparams2, tparams) + for i, res := range result { + result[i] = check.subst(posn.Pos(), res, unrenameMap, nil) + } + }() + } + } + // If we have more than 2 arguments, we may have arguments with named and unnamed types. // If that is the case, permutate params and args such that the arguments with named // types are first in the list. This doesn't affect type inference if all types are taken diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 0cce46ac46f..53247a35851 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -21,6 +21,17 @@ func makeSubstMap(tpars []*TypeParam, targs []Type) substMap { return proj } +// makeRenameMap is like makeSubstMap, but creates a map used to rename type +// parameters in from with the type parameters in to. +func makeRenameMap(from, to []*TypeParam) substMap { + assert(len(from) == len(to)) + proj := make(substMap, len(from)) + for i, tpar := range from { + proj[tpar] = to[i] + } + return proj +} + func (m substMap) empty() bool { return len(m) == 0 } diff --git a/src/go/types/testdata/fixedbugs/issue48619.go2 b/src/go/types/testdata/fixedbugs/issue48619.go2 index d33040d78f0..72eea1ef596 100644 --- a/src/go/types/testdata/fixedbugs/issue48619.go2 +++ b/src/go/types/testdata/fixedbugs/issue48619.go2 @@ -2,24 +2,19 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This issue is still open: -// - the error messages could be better or are incorrect -// - unification fails due to stack overflow that is caught - package p func f[P any](a, _ P) { var x int // TODO(gri) these error messages, while correct, could be better - f(a, x /* ERROR type int of x does not match P */) + f(a, x /* ERROR type int of x does not match inferred type P for P */) f(x, a /* ERROR type P of a does not match inferred type int for P */) } func g[P any](a, b P) { g(a, b) - // TODO(gri) these error messages are incorrect because the code is valid - g(&a, & /* ERROR type \*P of &b does not match inferred type \*P for P */ b) - g([]P{}, [ /* ERROR type \[\]P of \(\[\]P literal\) does not match inferred type \[\]P for P */ ]P{}) + g(&a, &b) + g([]P{}, []P{}) // work-around: provide type argument explicitly g[*P](&a, &b) diff --git a/src/go/types/testdata/fixedbugs/issue48656.go2 b/src/go/types/testdata/fixedbugs/issue48656.go2 index 493f220e98d..0f60f47120a 100644 --- a/src/go/types/testdata/fixedbugs/issue48656.go2 +++ b/src/go/types/testdata/fixedbugs/issue48656.go2 @@ -2,14 +2,12 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This issue is still open: -// - the error messages are unclear -// - unification fails due to stack overflow that is caught - package p func f[P *Q, Q any](P, Q) { - // TODO(gri) these error messages are unclear - _ = f /* ERROR P does not match \*Q */ [P] - _ = f /* ERROR cannot infer P */ [*P] + _ = f[P] +} + +func f2[P /* ERROR instantiation cycle */ *Q, Q any](P, Q) { + _ = f2[*P] } diff --git a/src/go/types/testdata/fixedbugs/issue51158.go2 b/src/go/types/testdata/fixedbugs/issue51158.go2 new file mode 100644 index 00000000000..3edc5053820 --- /dev/null +++ b/src/go/types/testdata/fixedbugs/issue51158.go2 @@ -0,0 +1,18 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +// Type checking the following code should not cause an infinite recursion. +func f[M map[K]int, K comparable](m M) { + f(m) +} + +// Equivalent code using mutual recursion. +func f1[M map[K]int, K comparable](m M) { + f2(m) +} +func f2[M map[K]int, K comparable](m M) { + f1(m) +}