]> Cypherpunks.ru repositories - gostls13.git/commitdiff
go/types, types2: consider type parameters for cycle detection
authorRobert Griesemer <gri@golang.org>
Sat, 15 Jan 2022 01:42:20 +0000 (17:42 -0800)
committerRobert Griesemer <gri@golang.org>
Mon, 24 Jan 2022 21:27:28 +0000 (21:27 +0000)
In validType, when we see an instantiated type, proceed as with
non-generic types but provide an environment in which to look up
the values (the corresponding type arguments) of type parameters
of the instantiated type. For each type parameter for which there
is a type argument, proceed with validating that type argument.
This corresponds to applying validType to the instantiated type
without actually instantiating the type (and running into infinite
instantiations in case of invalid recursive types).

Also, when creating a type instance, use the correct source position
for the instance (the start of the qualified identifier if we have an
imported type).

Fixes #48962.

Change-Id: I196c78bf066e4a56284d53368b2eb71bd8d8a780
Reviewed-on: https://go-review.googlesource.com/c/go/+/379414
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
21 files changed:
src/cmd/compile/internal/types2/testdata/check/issues.go2
src/cmd/compile/internal/types2/testdata/check/typeinst.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue39634.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue39938.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue48951.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue48962.go2 [new file with mode: 0644]
src/cmd/compile/internal/types2/testdata/fixedbugs/issue49043.go2
src/cmd/compile/internal/types2/typexpr.go
src/cmd/compile/internal/types2/validtype.go
src/go/types/testdata/check/issues.go2
src/go/types/testdata/check/typeinst.go2
src/go/types/testdata/fixedbugs/issue39634.go2
src/go/types/testdata/fixedbugs/issue39938.go2
src/go/types/testdata/fixedbugs/issue48951.go2
src/go/types/testdata/fixedbugs/issue48962.go2 [new file with mode: 0644]
src/go/types/testdata/fixedbugs/issue49043.go2
src/go/types/typexpr.go
src/go/types/validtype.go
test/typeparam/issue48962.dir/a.go [new file with mode: 0644]
test/typeparam/issue48962.dir/b.go [new file with mode: 0644]
test/typeparam/issue48962.go

index 5b6eebd4fd2eff93d87454c12a045886f72ea853..0b80939653865f24f35db69520b44d002ce74644 100644 (file)
@@ -145,8 +145,8 @@ type List3[TElem any] struct {
 }
 
 // Infinite generic type declarations must lead to an error.
-type inf1 /* ERROR illegal cycle */ [T any] struct{ _ inf1[T] }
-type inf2 /* ERROR illegal cycle */ [T any] struct{ inf2[T] }
+type inf1[T any] struct{ _ inf1 /* ERROR illegal cycle */ [T] }
+type inf2[T any] struct{ inf2 /* ERROR illegal cycle */ [T] }
 
 // The implementation of conversions T(x) between integers and floating-point
 // numbers checks that both T and x have either integer or floating-point
index a3d1b5e28fb52945b455c6b9f3267b4557b04fb7..0e6dc0a98f328db98796fdee7fe0c47e8ef9d8bd 100644 (file)
@@ -58,5 +58,5 @@ var _ T3[int] = T3[int](List[int]{1, 2, 3})
 
 // Self-recursive generic types are not permitted
 
-type self1 /* ERROR illegal cycle */ [P any] self1[P]
+type self1[P any] self1 /* ERROR illegal cycle */ [P]
 type self2[P any] *self2[P] // this is ok
index c56f23918d155ba14f76c680a175522252bfec34..b408dd70039597ffa468bbf91419f4e123d25e21 100644 (file)
@@ -37,7 +37,7 @@ func main7() { var _ foo7 = x7[int]{} }
 // func main8() {}
 
 // crash 9
-type foo9 /* ERROR illegal cycle */ [A any] interface { foo9[A] }
+type foo9[A any] interface { foo9 /* ERROR illegal cycle */ [A] }
 func _() { var _ = new(foo9[int]) }
 
 // crash 12
index 114646786d5b74324413b74cbbca0c211f3c6cb2..6bc928484956466e8d8467c493dab2882d73f70f 100644 (file)
@@ -2,22 +2,20 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Check "infinite expansion" cycle errors across instantiated types.
-// We can't detect these errors anymore at the moment. See #48962 for
-// details.
-
 package p
 
+// All but E2 and E5 provide an "indirection" and break infinite expansion of a type.
 type E0[P any] []P
 type E1[P any] *P
 type E2[P any] struct{ _ P }
 type E3[P any] struct{ _ *P }
+type E5[P any] struct{ _ [10]P }
 
-type T0 /* illegal cycle */ struct {
+type T0 struct {
         _ E0[T0]
 }
 
-type T0_ /* illegal cycle */ struct {
+type T0_ struct {
         E0[T0_]
 }
 
@@ -25,7 +23,7 @@ type T1 struct {
         _ E1[T1]
 }
 
-type T2 /* illegal cycle */ struct {
+type T2 /* ERROR illegal cycle */ struct {
         _ E2[T2]
 }
 
@@ -33,20 +31,24 @@ type T3 struct {
         _ E3[T3]
 }
 
-// some more complex cases
-
-type T4 /* illegal cycle */ struct {
-       _ E0[E2[T4]]
-}
+type T4 /* ERROR illegal cycle */ [10]E5[T4]
 
 type T5 struct {
-       _ E0[E2[E0[E1[E2[[10]T5]]]]]
+       _ E0[E2[T5]]
 }
 
-type T6 /* illegal cycle */ struct {
-       _ E0[[10]E2[E0[E2[E2[T6]]]]]
+type T6 struct {
+       _ E0[E2[E0[E1[E2[[10]T6]]]]]
 }
 
 type T7 struct {
-       _ E0[[]E2[E0[E2[E2[T6]]]]]
+       _ E0[[10]E2[E0[E2[E2[T7]]]]]
 }
+
+type T8 struct {
+       _ E0[[]E2[E0[E2[E2[T8]]]]]
+}
+
+type T9 /* ERROR illegal cycle */ [10]E2[E5[E2[T9]]]
+
+type T10 [10]E2[E5[E2[func(T10)]]]
index cf02cc130aa6cb44089d3abedf05734ea6fd0d42..a9365281ee8abd599de7cd1fe0d3827c684b9310 100644 (file)
@@ -5,17 +5,17 @@
 package p
 
 type (
-        A1 /* ERROR illegal cycle */ [P any] [10]A1[P]
-        A2 /* ERROR illegal cycle */ [P any] [10]A2[*P]
+        A1[P any] [10]A1 /* ERROR illegal cycle */ [P]
+        A2[P any] [10]A2 /* ERROR illegal cycle */ [*P]
         A3[P any] [10]*A3[P]
 
         L1[P any] []L1[P]
 
-        S1 /* ERROR illegal cycle */ [P any] struct{ f S1[P] }
-        S2 /* ERROR illegal cycle */ [P any] struct{ f S2[*P] } // like example in issue
+        S1[P any] struct{ f S1 /* ERROR illegal cycle */ [P] }
+        S2[P any] struct{ f S2 /* ERROR illegal cycle */ [*P] } // like example in issue
         S3[P any] struct{ f *S3[P] }
 
-        I1 /* ERROR illegal cycle */ [P any] interface{ I1[P] }
-        I2 /* ERROR illegal cycle */ [P any] interface{ I2[*P] }
+        I1[P any] interface{ I1 /* ERROR illegal cycle */ [P] }
+        I2[P any] interface{ I2 /* ERROR illegal cycle */ [*P] }
         I3[P any] interface{ *I3 /* ERROR interface contains type constraints */ [P] }
 )
diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48962.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48962.go2
new file mode 100644 (file)
index 0000000..4270da1
--- /dev/null
@@ -0,0 +1,13 @@
+// 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 T0[P any] struct {
+       f P
+}
+
+type T1 /* ERROR illegal cycle */ struct {
+       _ T0[T1]
+}
index c37b0f126783f6f0e3fc8ab5d0c088e9d96e0ecf..a360457d9ffd2dd30a97f43d2d55263c163e3a87 100644 (file)
@@ -6,13 +6,13 @@ package p
 
 // The example from the issue.
 type (
-       N /* ERROR illegal cycle */ [P any] M[P]
-       M[P any] N[P]
+       N[P any] M /* ERROR illegal cycle */ [P]
+       M[P any] N /* ERROR illegal cycle */ [P]
 )
 
 // A slightly more complicated case.
 type (
-       A /* ERROR illegal cycle */ [P any] B[P]
+       A[P any] B /* ERROR illegal cycle */ [P]
        B[P any] C[P]
        C[P any] A[P]
 )
index 580b53d3c7c30a6a67a2aee04b1c87b6fa429279..0c7bd626434962b1121cd045300589462a5ef5b0 100644 (file)
@@ -440,7 +440,8 @@ func (check *Checker) instantiatedType(x syntax.Expr, xlist []syntax.Expr, def *
        // validation below. Ensure that the validation (and resulting errors) runs
        // for each instantiated type in the source.
        if inst == nil {
-               tname := NewTypeName(x.Pos(), orig.obj.pkg, orig.obj.name, nil)
+               // x may be a selector for an imported type; use its start pos rather than x.Pos().
+               tname := NewTypeName(syntax.StartPos(x), orig.obj.pkg, orig.obj.name, nil)
                inst = check.newNamed(tname, orig, nil, nil, nil) // underlying, methods and tparams are set when named is resolved
                inst.targs = newTypeList(targs)
                inst = ctxt.update(h, orig, targs, inst).(*Named)
index 101a8b39454d6cf1db2ced63d13f95262fbbba37..c508eadc7ceefa7b9fcb1f049397573ccb8b7609 100644 (file)
@@ -10,12 +10,17 @@ package types2
 // (Cycles involving alias types, as in "type A = [10]A" are detected
 // earlier, via the objDecl cycle detection mechanism.)
 func (check *Checker) validType(typ *Named) {
-       check.validType0(typ, nil)
+       check.validType0(typ, nil, nil)
 }
 
 type typeInfo uint
 
-func (check *Checker) validType0(typ Type, path []Object) typeInfo {
+// validType0 checks if the given type is valid. If typ is a type parameter
+// its value is looked up in the provided environment. The environment is
+// nil if typ is not part of (the RHS of) an instantiated type, in that case
+// any type parameter encountered must be from an enclosing function and can
+// be ignored. The path is the list of type names that lead to the current typ.
+func (check *Checker) validType0(typ Type, env *tparamEnv, path []Object) typeInfo {
        const (
                unknown typeInfo = iota
                marked
@@ -24,43 +29,39 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
        )
 
        switch t := typ.(type) {
+       case nil:
+               // We should never see a nil type but be conservative and panic
+               // only in debug mode.
+               if debug {
+                       panic("validType0(nil)")
+               }
+
        case *Array:
-               return check.validType0(t.elem, path)
+               return check.validType0(t.elem, env, path)
 
        case *Struct:
                for _, f := range t.fields {
-                       if check.validType0(f.typ, path) == invalid {
+                       if check.validType0(f.typ, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Union:
                for _, t := range t.terms {
-                       if check.validType0(t.typ, path) == invalid {
+                       if check.validType0(t.typ, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Interface:
                for _, etyp := range t.embeddeds {
-                       if check.validType0(etyp, path) == invalid {
+                       if check.validType0(etyp, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Named:
-               // If t is parameterized, we should be considering the instantiated (expanded)
-               // form of t, but in general we can't with this algorithm: if t is an invalid
-               // type it may be so because it infinitely expands through a type parameter.
-               // Instantiating such a type would lead to an infinite sequence of instantiations.
-               // In general, we need "type flow analysis" to recognize those cases.
-               // Example: type A[T any] struct{ x A[*T] } (issue #48951)
-               // In this algorithm we always only consider the original, uninstantiated type.
-               // This won't recognize some invalid cases with parameterized types, but it
-               // will terminate.
-               t = t.orig
-
-               // don't report a 2nd error if we already know the type is invalid
+               // Don't report a 2nd error if we already know the type is invalid
                // (e.g., if a cycle was detected earlier, via under).
                if t.underlying == Typ[Invalid] {
                        check.infoMap[t] = invalid
@@ -70,32 +71,77 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
                switch check.infoMap[t] {
                case unknown:
                        check.infoMap[t] = marked
-                       check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj))
+                       check.infoMap[t] = check.validType0(t.orig.fromRHS, env.push(t), append(path, t.obj))
                case marked:
-                       // cycle detected
+                       // We have seen type t before and thus must have a cycle.
+                       check.infoMap[t] = invalid
+                       // t cannot be in an imported package otherwise that package
+                       // would have reported a type cycle and couldn't have been
+                       // imported in the first place.
+                       assert(t.obj.pkg == check.pkg)
+                       t.underlying = Typ[Invalid] // t is in the current package (no race possibilty)
+                       // Find the starting point of the cycle and report it.
                        for i, tn := range path {
-                               // Even though validType now can hande cycles through external
-                               // types, we can't have cycles through external types because
-                               // no such types are detected yet.
-                               // TODO(gri) Remove this check once we can detect such cycles,
-                               //           and adjust cycleError accordingly.
-                               if t.obj.pkg != check.pkg {
-                                       panic("type cycle via package-external type")
-                               }
                                if tn == t.obj {
                                        check.cycleError(path[i:])
-                                       check.infoMap[t] = invalid
-                                       // don't modify imported types (leads to race condition, see #35049)
-                                       if t.obj.pkg == check.pkg {
-                                               t.underlying = Typ[Invalid]
-                                       }
                                        return invalid
                                }
                        }
                        panic("cycle start not found")
                }
                return check.infoMap[t]
+
+       case *TypeParam:
+               // A type parameter stands for the type (argument) it was instantiated with.
+               // Check the corresponding type argument for validity if we have one.
+               if env != nil {
+                       if targ := env.tmap[t]; targ != nil {
+                               // Type arguments found in targ must be looked
+                               // up in the enclosing environment env.link.
+                               return check.validType0(targ, env.link, path)
+                       }
+               }
        }
 
        return valid
 }
+
+// A tparamEnv provides the environment for looking up the type arguments
+// with which type parameters for a given instance were instantiated.
+// If we don't have an instance, the corresponding tparamEnv is nil.
+type tparamEnv struct {
+       tmap substMap
+       link *tparamEnv
+}
+
+func (env *tparamEnv) push(typ *Named) *tparamEnv {
+       // If typ is not an instantiated type there are no typ-specific
+       // type parameters to look up and we don't need an environment.
+       targs := typ.TypeArgs()
+       if targs == nil {
+               return nil // no instance => nil environment
+       }
+
+       // Populate tmap: remember the type argument for each type parameter.
+       // We cannot use makeSubstMap because the number of type parameters
+       // and arguments may not match due to errors in the source (too many
+       // or too few type arguments). Populate tmap "manually".
+       tparams := typ.TypeParams()
+       n, m := targs.Len(), tparams.Len()
+       if n > m {
+               n = m // too many targs
+       }
+       tmap := make(substMap, n)
+       for i := 0; i < n; i++ {
+               tmap[tparams.At(i)] = targs.At(i)
+       }
+
+       return &tparamEnv{tmap: tmap, link: env}
+}
+
+// TODO(gri) Alternative implementation:
+//           We may not need to build a stack of environments to
+//           look up the type arguments for type parameters. The
+//           same information should be available via the path:
+//           We should be able to just walk the path backwards
+//           and find the type arguments in the instance objects.
index cec1ccb0cc8effa9ec30d13de9021bdcc920e41f..a11bcaac4bca8b41fc6b4cf4a358f34a5dbaa7c0 100644 (file)
@@ -145,8 +145,8 @@ type List3[TElem any] struct {
 }
 
 // Infinite generic type declarations must lead to an error.
-type inf1 /* ERROR illegal cycle */ [T any] struct{ _ inf1[T] }
-type inf2 /* ERROR illegal cycle */ [T any] struct{ inf2[T] }
+type inf1[T any] struct{ _ inf1 /* ERROR illegal cycle */ [T] }
+type inf2[T any] struct{ inf2 /* ERROR illegal cycle */ [T] }
 
 // The implementation of conversions T(x) between integers and floating-point
 // numbers checks that both T and x have either integer or floating-point
index 65481202e4c4e8ccdff904a66c5d7ef48b5da1f9..6423cb801f9971d69ea6f07eed270aca3b220f9a 100644 (file)
@@ -58,5 +58,5 @@ var _ T3[int] = T3[int](List[int]{1, 2, 3})
 
 // Self-recursive generic types are not permitted
 
-type self1 /* ERROR illegal cycle */ [P any] self1[P]
+type self1[P any] self1 /* ERROR illegal cycle */ [P]
 type self2[P any] *self2[P] // this is ok
index 2de2f4378aac0a8206b0ec2d99fd8eab719bbc26..34ab654f1cf1eef5678ff3560d4f78fa666dc4cd 100644 (file)
@@ -37,7 +37,7 @@ func main7() { var _ foo7 = x7[int]{} }
 // func main8() {}
 
 // crash 9
-type foo9 /* ERROR illegal cycle */ [A any] interface { foo9[A] }
+type foo9[A any] interface { foo9 /* ERROR illegal cycle */ [A] }
 func _() { var _ = new(foo9[int]) }
 
 // crash 12
index 114646786d5b74324413b74cbbca0c211f3c6cb2..6bc928484956466e8d8467c493dab2882d73f70f 100644 (file)
@@ -2,22 +2,20 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Check "infinite expansion" cycle errors across instantiated types.
-// We can't detect these errors anymore at the moment. See #48962 for
-// details.
-
 package p
 
+// All but E2 and E5 provide an "indirection" and break infinite expansion of a type.
 type E0[P any] []P
 type E1[P any] *P
 type E2[P any] struct{ _ P }
 type E3[P any] struct{ _ *P }
+type E5[P any] struct{ _ [10]P }
 
-type T0 /* illegal cycle */ struct {
+type T0 struct {
         _ E0[T0]
 }
 
-type T0_ /* illegal cycle */ struct {
+type T0_ struct {
         E0[T0_]
 }
 
@@ -25,7 +23,7 @@ type T1 struct {
         _ E1[T1]
 }
 
-type T2 /* illegal cycle */ struct {
+type T2 /* ERROR illegal cycle */ struct {
         _ E2[T2]
 }
 
@@ -33,20 +31,24 @@ type T3 struct {
         _ E3[T3]
 }
 
-// some more complex cases
-
-type T4 /* illegal cycle */ struct {
-       _ E0[E2[T4]]
-}
+type T4 /* ERROR illegal cycle */ [10]E5[T4]
 
 type T5 struct {
-       _ E0[E2[E0[E1[E2[[10]T5]]]]]
+       _ E0[E2[T5]]
 }
 
-type T6 /* illegal cycle */ struct {
-       _ E0[[10]E2[E0[E2[E2[T6]]]]]
+type T6 struct {
+       _ E0[E2[E0[E1[E2[[10]T6]]]]]
 }
 
 type T7 struct {
-       _ E0[[]E2[E0[E2[E2[T6]]]]]
+       _ E0[[10]E2[E0[E2[E2[T7]]]]]
 }
+
+type T8 struct {
+       _ E0[[]E2[E0[E2[E2[T8]]]]]
+}
+
+type T9 /* ERROR illegal cycle */ [10]E2[E5[E2[T9]]]
+
+type T10 [10]E2[E5[E2[func(T10)]]]
index cf02cc130aa6cb44089d3abedf05734ea6fd0d42..a9365281ee8abd599de7cd1fe0d3827c684b9310 100644 (file)
@@ -5,17 +5,17 @@
 package p
 
 type (
-        A1 /* ERROR illegal cycle */ [P any] [10]A1[P]
-        A2 /* ERROR illegal cycle */ [P any] [10]A2[*P]
+        A1[P any] [10]A1 /* ERROR illegal cycle */ [P]
+        A2[P any] [10]A2 /* ERROR illegal cycle */ [*P]
         A3[P any] [10]*A3[P]
 
         L1[P any] []L1[P]
 
-        S1 /* ERROR illegal cycle */ [P any] struct{ f S1[P] }
-        S2 /* ERROR illegal cycle */ [P any] struct{ f S2[*P] } // like example in issue
+        S1[P any] struct{ f S1 /* ERROR illegal cycle */ [P] }
+        S2[P any] struct{ f S2 /* ERROR illegal cycle */ [*P] } // like example in issue
         S3[P any] struct{ f *S3[P] }
 
-        I1 /* ERROR illegal cycle */ [P any] interface{ I1[P] }
-        I2 /* ERROR illegal cycle */ [P any] interface{ I2[*P] }
+        I1[P any] interface{ I1 /* ERROR illegal cycle */ [P] }
+        I2[P any] interface{ I2 /* ERROR illegal cycle */ [*P] }
         I3[P any] interface{ *I3 /* ERROR interface contains type constraints */ [P] }
 )
diff --git a/src/go/types/testdata/fixedbugs/issue48962.go2 b/src/go/types/testdata/fixedbugs/issue48962.go2
new file mode 100644 (file)
index 0000000..4270da1
--- /dev/null
@@ -0,0 +1,13 @@
+// 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 T0[P any] struct {
+       f P
+}
+
+type T1 /* ERROR illegal cycle */ struct {
+       _ T0[T1]
+}
index c37b0f126783f6f0e3fc8ab5d0c088e9d96e0ecf..a360457d9ffd2dd30a97f43d2d55263c163e3a87 100644 (file)
@@ -6,13 +6,13 @@ package p
 
 // The example from the issue.
 type (
-       N /* ERROR illegal cycle */ [P any] M[P]
-       M[P any] N[P]
+       N[P any] M /* ERROR illegal cycle */ [P]
+       M[P any] N /* ERROR illegal cycle */ [P]
 )
 
 // A slightly more complicated case.
 type (
-       A /* ERROR illegal cycle */ [P any] B[P]
+       A[P any] B /* ERROR illegal cycle */ [P]
        B[P any] C[P]
        C[P any] A[P]
 )
index 82de90b67a668a8f610ed5fa7c3c598cd2321adc..1e629e3fdb242a94826734bf086ef7aaa6d048b5 100644 (file)
@@ -425,7 +425,8 @@ func (check *Checker) instantiatedType(ix *typeparams.IndexExpr, def *Named) (re
        // validation below. Ensure that the validation (and resulting errors) runs
        // for each instantiated type in the source.
        if inst == nil {
-               tname := NewTypeName(ix.X.Pos(), orig.obj.pkg, orig.obj.name, nil)
+               // x may be a selector for an imported type; use its start pos rather than x.Pos().
+               tname := NewTypeName(ix.Pos(), orig.obj.pkg, orig.obj.name, nil)
                inst = check.newNamed(tname, orig, nil, nil, nil) // underlying, methods and tparams are set when named is resolved
                inst.targs = newTypeList(targs)
                inst = ctxt.update(h, orig, targs, inst).(*Named)
index 865dc9528f2f790ee0b69d77e01245a7dd501947..c4ec2f2e0ae8051a5108be391364191e844b3d78 100644 (file)
@@ -10,12 +10,17 @@ package types
 // (Cycles involving alias types, as in "type A = [10]A" are detected
 // earlier, via the objDecl cycle detection mechanism.)
 func (check *Checker) validType(typ *Named) {
-       check.validType0(typ, nil)
+       check.validType0(typ, nil, nil)
 }
 
 type typeInfo uint
 
-func (check *Checker) validType0(typ Type, path []Object) typeInfo {
+// validType0 checks if the given type is valid. If typ is a type parameter
+// its value is looked up in the provided environment. The environment is
+// nil if typ is not part of (the RHS of) an instantiated type, in that case
+// any type parameter encountered must be from an enclosing function and can
+// be ignored. The path is the list of type names that lead to the current typ.
+func (check *Checker) validType0(typ Type, env *tparamEnv, path []Object) typeInfo {
        const (
                unknown typeInfo = iota
                marked
@@ -24,43 +29,39 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
        )
 
        switch t := typ.(type) {
+       case nil:
+               // We should never see a nil type but be conservative and panic
+               // only in debug mode.
+               if debug {
+                       panic("validType0(nil)")
+               }
+
        case *Array:
-               return check.validType0(t.elem, path)
+               return check.validType0(t.elem, env, path)
 
        case *Struct:
                for _, f := range t.fields {
-                       if check.validType0(f.typ, path) == invalid {
+                       if check.validType0(f.typ, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Union:
                for _, t := range t.terms {
-                       if check.validType0(t.typ, path) == invalid {
+                       if check.validType0(t.typ, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Interface:
                for _, etyp := range t.embeddeds {
-                       if check.validType0(etyp, path) == invalid {
+                       if check.validType0(etyp, env, path) == invalid {
                                return invalid
                        }
                }
 
        case *Named:
-               // If t is parameterized, we should be considering the instantiated (expanded)
-               // form of t, but in general we can't with this algorithm: if t is an invalid
-               // type it may be so because it infinitely expands through a type parameter.
-               // Instantiating such a type would lead to an infinite sequence of instantiations.
-               // In general, we need "type flow analysis" to recognize those cases.
-               // Example: type A[T any] struct{ x A[*T] } (issue #48951)
-               // In this algorithm we always only consider the original, uninstantiated type.
-               // This won't recognize some invalid cases with parameterized types, but it
-               // will terminate.
-               t = t.orig
-
-               // don't report a 2nd error if we already know the type is invalid
+               // Don't report a 2nd error if we already know the type is invalid
                // (e.g., if a cycle was detected earlier, via under).
                if t.underlying == Typ[Invalid] {
                        check.infoMap[t] = invalid
@@ -70,32 +71,77 @@ func (check *Checker) validType0(typ Type, path []Object) typeInfo {
                switch check.infoMap[t] {
                case unknown:
                        check.infoMap[t] = marked
-                       check.infoMap[t] = check.validType0(t.fromRHS, append(path, t.obj))
+                       check.infoMap[t] = check.validType0(t.orig.fromRHS, env.push(t), append(path, t.obj))
                case marked:
-                       // cycle detected
+                       // We have seen type t before and thus must have a cycle.
+                       check.infoMap[t] = invalid
+                       // t cannot be in an imported package otherwise that package
+                       // would have reported a type cycle and couldn't have been
+                       // imported in the first place.
+                       assert(t.obj.pkg == check.pkg)
+                       t.underlying = Typ[Invalid] // t is in the current package (no race possibilty)
+                       // Find the starting point of the cycle and report it.
                        for i, tn := range path {
-                               // Even though validType now can hande cycles through external
-                               // types, we can't have cycles through external types because
-                               // no such types are detected yet.
-                               // TODO(gri) Remove this check once we can detect such cycles,
-                               //           and adjust cycleError accordingly.
-                               if t.obj.pkg != check.pkg {
-                                       panic("type cycle via package-external type")
-                               }
                                if tn == t.obj {
                                        check.cycleError(path[i:])
-                                       check.infoMap[t] = invalid
-                                       // don't modify imported types (leads to race condition, see #35049)
-                                       if t.obj.pkg == check.pkg {
-                                               t.underlying = Typ[Invalid]
-                                       }
                                        return invalid
                                }
                        }
                        panic("cycle start not found")
                }
                return check.infoMap[t]
+
+       case *TypeParam:
+               // A type parameter stands for the type (argument) it was instantiated with.
+               // Check the corresponding type argument for validity if we have one.
+               if env != nil {
+                       if targ := env.tmap[t]; targ != nil {
+                               // Type arguments found in targ must be looked
+                               // up in the enclosing environment env.link.
+                               return check.validType0(targ, env.link, path)
+                       }
+               }
        }
 
        return valid
 }
+
+// A tparamEnv provides the environment for looking up the type arguments
+// with which type parameters for a given instance were instantiated.
+// If we don't have an instance, the corresponding tparamEnv is nil.
+type tparamEnv struct {
+       tmap substMap
+       link *tparamEnv
+}
+
+func (env *tparamEnv) push(typ *Named) *tparamEnv {
+       // If typ is not an instantiated type there are no typ-specific
+       // type parameters to look up and we don't need an environment.
+       targs := typ.TypeArgs()
+       if targs == nil {
+               return nil // no instance => nil environment
+       }
+
+       // Populate tmap: remember the type argument for each type parameter.
+       // We cannot use makeSubstMap because the number of type parameters
+       // and arguments may not match due to errors in the source (too many
+       // or too few type arguments). Populate tmap "manually".
+       tparams := typ.TypeParams()
+       n, m := targs.Len(), tparams.Len()
+       if n > m {
+               n = m // too many targs
+       }
+       tmap := make(substMap, n)
+       for i := 0; i < n; i++ {
+               tmap[tparams.At(i)] = targs.At(i)
+       }
+
+       return &tparamEnv{tmap: tmap, link: env}
+}
+
+// TODO(gri) Alternative implementation:
+//           We may not need to build a stack of environments to
+//           look up the type arguments for type parameters. The
+//           same information should be available via the path:
+//           We should be able to just walk the path backwards
+//           and find the type arguments in the instance objects.
diff --git a/test/typeparam/issue48962.dir/a.go b/test/typeparam/issue48962.dir/a.go
new file mode 100644 (file)
index 0000000..a6d2734
--- /dev/null
@@ -0,0 +1,12 @@
+// 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 a
+
+type (
+       A[P any]               [10]P
+       S[P any]               struct{ f P }
+       P[P any]               *P
+       M[K comparable, V any] map[K]V
+)
diff --git a/test/typeparam/issue48962.dir/b.go b/test/typeparam/issue48962.dir/b.go
new file mode 100644 (file)
index 0000000..a49f55d
--- /dev/null
@@ -0,0 +1,51 @@
+// 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 b
+
+import "a"
+
+type (
+       lA[P any]               [10]P
+       lS[P any]               struct{ f P }
+       lP[P any]               *P
+       lM[K comparable, V any] map[K]V
+)
+
+// local cycles
+type (
+       A  lA[A]            // ERROR "invalid recursive type"
+       S  lS[S]            // ERROR "invalid recursive type"
+       P  lP[P]            // ok (indirection through lP)
+       M1 lM[int, M1]      // ok (indirection through lM)
+       M2 lM[lA[byte], M2] // ok (indirection through lM)
+
+       A2 lA[lS[lP[A2]]] // ok (indirection through lP)
+       A3 lA[lS[lS[A3]]] // ERROR "invalid recursive type"
+)
+
+// cycles through imported types
+type (
+       Ai  a.A[Ai]             // ERROR "invalid recursive type"
+       Si  a.S[Si]             // ERROR "invalid recursive type"
+       Pi  a.P[Pi]             // ok (indirection through a.P)
+       M1i a.M[int, M1i]       // ok (indirection through a.M)
+       M2i a.M[a.A[byte], M2i] // ok (indirection through a.M)
+
+       A2i a.A[a.S[a.P[A2i]]] // ok (indirection through a.P)
+       A3i a.A[a.S[a.S[A3i]]] // ERROR "invalid recursive type"
+
+       T2 a.S[T0[T2]] // ERROR "invalid recursive type"
+       T3 T0[Ai]      // no follow-on error here
+)
+
+// test case from issue
+
+type T0[P any] struct {
+       f P
+}
+
+type T1 struct { // ERROR "invalid recursive type"
+       _ T0[T1]
+}
index de9a23cdd2185ea8590f4d20c2cd8ef250f850e4..326d67b49a8539996a38132d4f722b4682983ca9 100644 (file)
@@ -1,15 +1,7 @@
-// errorcheck -G=3
+// errorcheckdir -G=3
 
-// Copyright 2021 The Go Authors. All rights reserved.
+// 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 T0[P any] struct { // ERROR "invalid recursive type"
-       f P
-}
-
-type T1 struct {
-       _ T0[T1]
-}
+package ignored