Fixes #65711.
Change-Id: I3196b7d053c9868b74c53623526f2da0ab878f53
Reviewed-on: https://go-review.googlesource.com/c/go/+/567976
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
nextID uint64 // unique Id for type parameters (first valid Id is 1)
objMap map[Object]*declInfo // maps package-level objects and (non-interface) methods to declaration info
impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
- valids instanceLookup // valid *Named (incl. instantiated) types per the validType check
+ // see TODO in validtype.go
+ // valids instanceLookup // valid *Named (incl. instantiated) types per the validType check
// pkgPathMap maps package names to the set of distinct import paths we've
// seen for that name, anywhere in the import graph. It is used for
}
case *Named:
- // Exit early if we already know t is valid.
- // This is purely an optimization but it prevents excessive computation
- // times in pathological cases such as testdata/fixedbugs/issue6977.go.
- // (Note: The valids map could also be allocated locally, once for each
- // validType call.)
- if check.valids.lookup(t) != nil {
- break
- }
+ // TODO(gri) The optimization below is incorrect (see go.dev/issue/65711):
+ // in that issue `type A[P any] [1]P` is a valid type on its own
+ // and the (uninstantiated) A is recorded in check.valids. As a
+ // consequence, when checking the remaining declarations, which
+ // are not valid, the validity check ends prematurely because A
+ // is considered valid, even though its validity depends on the
+ // type argument provided to it.
+ //
+ // A correct optimization is important for pathological cases.
+ // Keep code around for reference until we found an optimization.
+ //
+ // // Exit early if we already know t is valid.
+ // // This is purely an optimization but it prevents excessive computation
+ // // times in pathological cases such as testdata/fixedbugs/issue6977.go.
+ // // (Note: The valids map could also be allocated locally, once for each
+ // // validType call.)
+ // if check.valids.lookup(t) != nil {
+ // break
+ // }
// Don't report a 2nd error if we already know the type is invalid
// (e.g., if a cycle was detected earlier, via under).
return false
}
- check.valids.add(t) // t is valid
+ // see TODO above
+ // check.valids.add(t) // t is valid
case *TypeParam:
// A type parameter stands for the type (argument) it was instantiated with.
nextID uint64 // unique Id for type parameters (first valid Id is 1)
objMap map[Object]*declInfo // maps package-level objects and (non-interface) methods to declaration info
impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
- valids instanceLookup // valid *Named (incl. instantiated) types per the validType check
+ // see TODO in validtype.go
+ // valids instanceLookup // valid *Named (incl. instantiated) types per the validType check
// pkgPathMap maps package names to the set of distinct import paths we've
// seen for that name, anywhere in the import graph. It is used for
}
case *Named:
- // Exit early if we already know t is valid.
- // This is purely an optimization but it prevents excessive computation
- // times in pathological cases such as testdata/fixedbugs/issue6977.go.
- // (Note: The valids map could also be allocated locally, once for each
- // validType call.)
- if check.valids.lookup(t) != nil {
- break
- }
+ // TODO(gri) The optimization below is incorrect (see go.dev/issue/65711):
+ // in that issue `type A[P any] [1]P` is a valid type on its own
+ // and the (uninstantiated) A is recorded in check.valids. As a
+ // consequence, when checking the remaining declarations, which
+ // are not valid, the validity check ends prematurely because A
+ // is considered valid, even though its validity depends on the
+ // type argument provided to it.
+ //
+ // A correct optimization is important for pathological cases.
+ // Keep code around for reference until we found an optimization.
+ //
+ // // Exit early if we already know t is valid.
+ // // This is purely an optimization but it prevents excessive computation
+ // // times in pathological cases such as testdata/fixedbugs/issue6977.go.
+ // // (Note: The valids map could also be allocated locally, once for each
+ // // validType call.)
+ // if check.valids.lookup(t) != nil {
+ // break
+ // }
// Don't report a 2nd error if we already know the type is invalid
// (e.g., if a cycle was detected earlier, via under).
return false
}
- check.valids.add(t) // t is valid
+ // see TODO above
+ // check.valids.add(t) // t is valid
case *TypeParam:
// A type parameter stands for the type (argument) it was instantiated with.
--- /dev/null
+// Copyright 2024 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 A[P any] [1]P
+
+type B[P any] A /* ERROR "invalid recursive type" */ [P]
+
+type C B[C]
+
+// test case from issue
+
+type Foo[T any] struct {
+ baz T
+}
+
+type Bar[T any] struct {
+ foo Foo /* ERROR "invalid recursive type" */ [T]
+}
+
+type Baz struct {
+ bar Bar[Baz]
+}
T8 interface { T7; T7 }
T9 interface { T8; T8 }
- T10 interface { T9; T9 }
- T11 interface { T10; T10 }
- T12 interface { T11; T11 }
- T13 interface { T12; T12 }
- T14 interface { T13; T13 }
- T15 interface { T14; T14 }
- T16 interface { T15; T15 }
- T17 interface { T16; T16 }
- T18 interface { T17; T17 }
- T19 interface { T18; T18 }
-
- T20 interface { T19; T19 }
- T21 interface { T20; T20 }
- T22 interface { T21; T21 }
- T23 interface { T22; T22 }
- T24 interface { T23; T23 }
- T25 interface { T24; T24 }
- T26 interface { T25; T25 }
- T27 interface { T26; T26 }
- T28 interface { T27; T27 }
- T29 interface { T28; T28 }
+ // TODO(gri) Enable this longer test once we have found a solution
+ // for the incorrect optimization in the validType check
+ // (see TODO in validtype.go).
+ // T10 interface { T9; T9 }
+ // T11 interface { T10; T10 }
+ // T12 interface { T11; T11 }
+ // T13 interface { T12; T12 }
+ // T14 interface { T13; T13 }
+ // T15 interface { T14; T14 }
+ // T16 interface { T15; T15 }
+ // T17 interface { T16; T16 }
+ // T18 interface { T17; T17 }
+ // T19 interface { T18; T18 }
+
+ // T20 interface { T19; T19 }
+ // T21 interface { T20; T20 }
+ // T22 interface { T21; T21 }
+ // T23 interface { T22; T22 }
+ // T24 interface { T23; T23 }
+ // T25 interface { T24; T24 }
+ // T26 interface { T25; T25 }
+ // T27 interface { T26; T26 }
+ // T28 interface { T27; T27 }
+ // T29 interface { T28; T28 }
)
// Verify that m is present.
-var x T29
+var x T9 // T29
var _ = x.m