]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: avoid infinitely recursive instantiation
authorRobert Findley <rfindley@google.com>
Sun, 13 Feb 2022 02:29:27 +0000 (21:29 -0500)
committerRobert Findley <rfindley@google.com>
Mon, 14 Feb 2022 02:34:05 +0000 (02:34 +0000)
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>

src/cmd/compile/internal/types2/infer.go
src/cmd/compile/internal/types2/subst.go
src/cmd/compile/internal/types2/testdata/fixedbugs/issue48619.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue48656.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue51158.go2 [new file with mode: 0644]
src/go/types/infer.go
src/go/types/subst.go
src/go/types/testdata/fixedbugs/issue48619.go2
src/go/types/testdata/fixedbugs/issue48656.go2
src/go/types/testdata/fixedbugs/issue51158.go2 [new file with mode: 0644]

index df87f8da4fb1aaa93caea50dc548412c4525729b..6259e287aeda074d4769c5fb6d4557839d649c78 100644 (file)
@@ -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
index f2e8fecc05bbf49aae74a5316fe8a7ab44f0f8a4..44a59f55fd8b4b3a4e1646cecff62cc9e17e75c2 100644 (file)
@@ -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
 }
index 3d4f1b4707f769daf0dc291716bf03e4abebc6ee..72eea1ef596204c3613d1706bd04045399648d66 100644 (file)
@@ -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)
index bea3dc14a0ec7662acfd560b0de96f2563026b14..0f60f47120a96b72fb8c5c603e803b7ee492e112 100644 (file)
@@ -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 (file)
index 0000000..3edc505
--- /dev/null
@@ -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)
+}
index b4b6b7801695710f393caff2606596cb8d45534c..18ec81edd438b0af5ce580d69a10a3a975b57aad 100644 (file)
@@ -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
index 0cce46ac46f0e7e66d7d1aac973057a9db06e387..53247a35851d0f0bc0912ca1c23263bbff6647bb 100644 (file)
@@ -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
 }
index d33040d78f075f7f404a80d592e20fb6a39d7659..72eea1ef596204c3613d1706bd04045399648d66 100644 (file)
@@ -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)
index 493f220e98db98e20f4fbaee4c3b8955c8be83e1..0f60f47120a96b72fb8c5c603e803b7ee492e112 100644 (file)
@@ -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 (file)
index 0000000..3edc505
--- /dev/null
@@ -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)
+}