go/types: ensure named types are expanded after type-checking

Rather than using Checker.later in newNamed, add a Checker.defTypes
field to track named types that have been created during type-checking,
and use this to expand named types as a final phase in type checking.

We have encountered several bugs related to infinite recursion while
expanding named types, because (I would argue) we have two conflicting
requirements in the type checker: ensuring that we eventually collapse
underlying chains, and yet allowing lazy substitution of the underlying
type in instances. The former is necessary for correctness, and to
ensure that we detect cycles during the type-checking pass. The latter
is necessary to allow infinitely expanding patterns of instances through
underlying or method definitions.

I believe this CL reconciles these conflicting requirements, by creating
a boundary between types that are encountered in the source during
type checking, and instances that are created by recursive evaluation.
At the end of the type checking pass, Checker.defTypes should contain
all possible origin types for instantiation. Once we compute the true
underlying for these origin types, any remaining instances that are
unresolved are guaranteed to have an origin with a valid underlying.
Therefore, we can return from the type-checking pass without calling
under() for these remaining instances.

Fixes #48703
Fixes #48974

Change-Id: I1474f514e2ab71c1ad4c3704fe32bfba11d59394
Reviewed-on: https://go-review.googlesource.com/c/go/+/356490
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

Author: findleyr

src/go/types/check.go

@@ -113,6 +113,7 @@ type Checker struct {
 	untyped  map[ast.Expr]exprInfo // map of expressions without final type
 	delayed  []func()              // stack of delayed action segments; segments are processed in FIFO order
 	objPath  []Object              // path of object dependencies during type inference (for cycle reporting)
+	defTypes []*Named              // defined types created during type checking, for final validation.
 
 	// context within which the current object is type-checked
 	// (valid only for the duration of type-checking a specific object)
@@ -269,6 +270,8 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 
 	check.processDelayed(0) // incl. all functions
 
+	check.expandDefTypes()
+
 	check.initOrder()
 
 	if !check.conf.DisableUnusedImportCheck {
@@ -285,6 +288,7 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 	check.pkgPathMap = nil
 	check.seenPkgMap = nil
 	check.recvTParamMap = nil
+	check.defTypes = nil
 
 	// TODO(rFindley) There's more memory we should release at this point.
 
@@ -306,6 +310,29 @@ func (check *Checker) processDelayed(top int) {
 	check.delayed = check.delayed[:top]
 }
 
+func (check *Checker) expandDefTypes() {
+	// Ensure that every defined type created in the course of type-checking has
+	// either non-*Named underlying, or is unresolved.
+	//
+	// This guarantees that we don't leak any types whose underlying is *Named,
+	// because any unresolved instances will lazily compute their underlying by
+	// substituting in the underlying of their origin. The origin must have
+	// either been imported or type-checked and expanded here, and in either case
+	// its underlying will be fully expanded.
+	for i := 0; i < len(check.defTypes); i++ {
+		n := check.defTypes[i]
+		switch n.underlying.(type) {
+		case nil:
+			if n.resolver == nil {
+				panic("nil underlying")
+			}
+		case *Named:
+			n.under() // n.under may add entries to check.defTypes
+		}
+		n.check = nil
+	}
+}
+
 func (check *Checker) record(x *operand) {
 	// convert x into a user-friendly set of values
 	// TODO(gri) this code can be simplified

src/go/types/named.go

@@ -65,22 +65,9 @@ func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tpar
 	if obj.typ == nil {
 		obj.typ = typ
 	}
-	// Ensure that typ is always expanded, at which point the check field can be
-	// nilled out.
-	//
-	// Note that currently we cannot nil out check inside typ.under(), because
-	// it's possible that typ is expanded multiple times.
-	//
-	// TODO(rFindley): clean this up so that under is the only function mutating
-	//                 named types.
+	// Ensure that typ is always expanded and sanity-checked.
 	if check != nil {
-		check.later(func() {
-			switch typ.under().(type) {
-			case *Named:
-				panic("unexpanded underlying type")
-			}
-			typ.check = nil
-		})
+		check.defTypes = append(check.defTypes, typ)
 	}
 	return typ
 }
@@ -241,6 +228,12 @@ func expandNamed(ctxt *Context, n *Named, instPos token.Pos) (tparams *TypeParam
 
 	check := n.check
 
+	if _, unexpanded := n.orig.underlying.(*Named); unexpanded {
+		// We should only get an unexpanded underlying here during type checking
+		// (for example, in recursive type declarations).
+		assert(check != nil)
+	}
+
 	// Mismatching arg and tparam length may be checked elsewhere.
 	if n.orig.tparams.Len() == n.targs.Len() {
 		// We must always have a context, to avoid infinite recursion.

src/go/types/testdata/fixedbugs/issue48703.go2

@@ -0,0 +1,27 @@
+// Copyright 2021 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
+
+import "unsafe"
+
+// The actual example from the issue.
+type List[P any] struct{}
+
+func (_ List[P]) m() (_ List[List[P]]) { return }
+
+// Other types of recursion through methods.
+type R[P any] int
+
+func (*R[R /* ERROR must be an identifier */ [int]]) m0() {}
+func (R[P]) m1(R[R[P]])                                   {}
+func (R[P]) m2(R[*P])                                     {}
+func (R[P]) m3([unsafe.Sizeof(new(R[P]))]int)             {}
+func (R[P]) m4([unsafe.Sizeof(new(R[R[P]]))]int)          {}
+
+// Mutual recursion
+type M[P any] int
+
+func (R[P]) m5(M[M[P]]) {}
+func (M[P]) m(R[R[P]])  {}

src/go/types/testdata/fixedbugs/issue48974.go2

@@ -0,0 +1,22 @@
+// Copyright 2021 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 Fooer interface {
+	Foo()
+}
+
+type Fooable[F Fooer] struct {
+	ptr F
+}
+
+func (f *Fooable[F]) Adapter() *Fooable[*FooerImpl[F]] {
+	return &Fooable[*FooerImpl[F]]{&FooerImpl[F]{}}
+}
+
+type FooerImpl[F Fooer] struct {
+}
+
+func (fi *FooerImpl[F]) Foo() {}