go/types, types2: avoid infinitely recursive instantiation

Type inference uses type parameter pointer identity to keep track of the
correspondence between type parameters and type arguments. However, this
technique can misidentify type parameters that are used in explicit type
arguments or function arguments, as in the recursive instantiation
below:

  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. This
CL does this for self-recursive instantiations.

Fixes #51158
Fixes #48656
Updates #48619

Change-Id: I54fe37f2a79c9d98950cf6a3602335db2896dc24
Reviewed-on: https://go-review.googlesource.com/c/go/+/385494
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

10 files changed, 212 insertions, 32 deletions

diff --git a/src/cmd/compile/internal/types2/infer.go b/src/cmd/compile/internal/types2/infer.go
index df87f8da4f..6259e287ae 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 f2e8fecc05..44a59f55fd 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 3d4f1b4707..72eea1ef59 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 bea3dc14a0..0f60f47120 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 0000000000..3edc505382
--- /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 b4b6b78016..18ec81edd4 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 0cce46ac46..53247a3585 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 d33040d78f..72eea1ef59 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 493f220e98..0f60f47120 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 0000000000..3edc505382
--- /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)
+}