go/types, types2: generalize cleanup phase after type checking

Use a cleanup method and simple registration mechanism
for types that need some final processing before the end
of type checking.

Use cleanup mechanism instead of expandDefTypes mechanism
for *Named types. There is no change in functionality here.

Use cleanup mechanism also for TypeParam and Interface types
to ensure that their check fields are nilled out at the end.

Introduce a simple constructor method for Interface types
to ensure that the cleanup method is always registered.

In go/types, add tracing code to Checker.checkFiles to match
types2.

Minor comment adjustments.

Fixes #51316.
Fixes #51326.

Change-Id: I7b2bd79cc419717982f3c918645af623c0e80d5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/387417
Trust: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: griesemer

src/cmd/compile/internal/types2/check.go

@@ -126,7 +126,7 @@ type Checker struct {
 	untyped  map[syntax.Expr]exprInfo // map of expressions without final type
 	delayed  []action                 // 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.
+	cleaners []cleaner                // list of types that may need a final cleanup at the end of type-checking
 
 	// environment within which the current object is type-checked (valid only
 	// for the duration of type-checking a specific object)
@@ -205,6 +205,16 @@ func (check *Checker) pop() Object {
 	return obj
 }
 
+type cleaner interface {
+	cleanup()
+}
+
+// needsCleanup records objects/types that implement the cleanup method
+// which will be called at the end of type-checking.
+func (check *Checker) needsCleanup(c cleaner) {
+	check.cleaners = append(check.cleaners, c)
+}
+
 // NewChecker returns a new Checker instance for a given package.
 // Package files may be added incrementally via checker.Files.
 func NewChecker(conf *Config, pkg *Package, info *Info) *Checker {
@@ -247,6 +257,8 @@ func (check *Checker) initFiles(files []*syntax.File) {
 	check.methods = nil
 	check.untyped = nil
 	check.delayed = nil
+	check.objPath = nil
+	check.cleaners = nil
 
 	// determine package name and collect valid files
 	pkg := check.pkg
@@ -315,8 +327,8 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) {
 	print("== processDelayed ==")
 	check.processDelayed(0) // incl. all functions
 
-	print("== expandDefTypes ==")
-	check.expandDefTypes()
+	print("== cleanup ==")
+	check.cleanup()
 
 	print("== initOrder ==")
 	check.initOrder()
@@ -344,7 +356,6 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) {
 	check.recvTParamMap = nil
 	check.brokenAliases = nil
 	check.unionTypeSets = nil
-	check.defTypes = nil
 	check.ctxt = nil
 
 	// TODO(gri) There's more memory we should release at this point.
@@ -372,27 +383,13 @@ 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
+// cleanup runs cleanup for all collected cleaners.
+func (check *Checker) cleanup() {
+	// Don't use a range clause since Named.cleanup may add more cleaners.
+	for i := 0; i < len(check.cleaners); i++ {
+		check.cleaners[i].cleanup()
 	}
+	check.cleaners = nil
 }
 
 func (check *Checker) record(x *operand) {

src/cmd/compile/internal/types2/interface.go

@@ -37,7 +37,7 @@ func NewInterfaceType(methods []*Func, embeddeds []Type) *Interface {
 	}
 
 	// set method receivers if necessary
-	typ := new(Interface)
+	typ := (*Checker)(nil).newInterface()
 	for _, m := range methods {
 		if sig := m.typ.(*Signature); sig.recv == nil {
 			sig.recv = NewVar(m.pos, m.pkg, "", typ)
@@ -54,6 +54,15 @@ func NewInterfaceType(methods []*Func, embeddeds []Type) *Interface {
 	return typ
 }
 
+// check may be nil
+func (check *Checker) newInterface() *Interface {
+	typ := &Interface{check: check}
+	if check != nil {
+		check.needsCleanup(typ)
+	}
+	return typ
+}
+
 // MarkImplicit marks the interface t as implicit, meaning this interface
 // corresponds to a constraint literal such as ~T or A|B without explicit
 // interface embedding. MarkImplicit should be called before any concurrent use
@@ -100,6 +109,11 @@ func (t *Interface) String() string   { return TypeString(t, nil) }
 // ----------------------------------------------------------------------------
 // Implementation
 
+func (t *Interface) cleanup() {
+	t.check = nil
+	t.embedPos = nil
+}
+
 func (check *Checker) interfaceType(ityp *Interface, iface *syntax.InterfaceType, def *Named) {
 	addEmbedded := func(pos syntax.Pos, typ Type) {
 		ityp.embeddeds = append(ityp.embeddeds, typ)
@@ -162,16 +176,10 @@ func (check *Checker) interfaceType(ityp *Interface, iface *syntax.InterfaceType
 	// (don't sort embeddeds: they must correspond to *embedPos entries)
 	sortMethods(ityp.methods)
 
-	// Compute type set with a non-nil *Checker as soon as possible
-	// to report any errors. Subsequent uses of type sets will use
-	// this computed type set and won't need to pass in a *Checker.
-	//
-	// Pin the checker to the interface type in the interim, in case the type set
-	// must be used before delayed funcs are processed (see issue #48234).
-	// TODO(rfindley): clean up use of *Checker with computeInterfaceTypeSet
-	ityp.check = check
+	// Compute type set as soon as possible to report any errors.
+	// Subsequent uses of type sets will use this computed type
+	// set and won't need to pass in a *Checker.
 	check.later(func() {
 		computeInterfaceTypeSet(check, iface.Pos(), ityp)
-		ityp.check = nil
 	}).describef(iface, "compute type set for %s", ityp)
 }

src/cmd/compile/internal/types2/named.go

@@ -72,11 +72,31 @@ func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tpar
 	}
 	// Ensure that typ is always expanded and sanity-checked.
 	if check != nil {
-		check.defTypes = append(check.defTypes, typ)
+		check.needsCleanup(typ)
 	}
 	return typ
 }
 
+func (t *Named) cleanup() {
+	// 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.
+	switch t.underlying.(type) {
+	case nil:
+		if t.resolver == nil {
+			panic("nil underlying")
+		}
+	case *Named:
+		t.under() // t.under may add entries to check.cleaners
+	}
+	t.check = nil
+}
+
 // Obj returns the type name for the declaration defining the named type t. For
 // instantiated types, this is the type name of the base type.
 func (t *Named) Obj() *TypeName { return t.orig.obj } // for non-instances this is the same as t.obj
@@ -360,11 +380,11 @@ func expandNamed(ctxt *Context, n *Named, instPos syntax.Pos) (tparams *TypePara
 				// that it wasn't substituted. In this case we need to create a new
 				// *Interface before modifying receivers.
 				if iface == n.orig.underlying {
-					iface = &Interface{
-						embeddeds: iface.embeddeds,
-						complete:  iface.complete,
-						implicit:  iface.implicit, // should be false but be conservative
-					}
+					old := iface
+					iface = check.newInterface()
+					iface.embeddeds = old.embeddeds
+					iface.complete = old.complete
+					iface.implicit = old.implicit // should be false but be conservative
 					underlying = iface
 				}
 				iface.methods = methods

src/cmd/compile/internal/types2/subst.go

@@ -160,7 +160,10 @@ func (subst *subster) typ(typ Type) Type {
 		methods, mcopied := subst.funcList(t.methods)
 		embeddeds, ecopied := subst.typeList(t.embeddeds)
 		if mcopied || ecopied {
-			iface := &Interface{embeddeds: embeddeds, implicit: t.implicit, complete: t.complete}
+			iface := subst.check.newInterface()
+			iface.embeddeds = embeddeds
+			iface.implicit = t.implicit
+			iface.complete = t.complete
 			// If we've changed the interface type, we may need to replace its
 			// receiver if the receiver type is the original interface. Receivers of
 			// *Named type are replaced during named type expansion.

src/cmd/compile/internal/types2/typeparam.go

@@ -36,6 +36,7 @@ func NewTypeParam(obj *TypeName, constraint Type) *TypeParam {
 	return (*Checker)(nil).newTypeParam(obj, constraint)
 }
 
+// check may be nil
 func (check *Checker) newTypeParam(obj *TypeName, constraint Type) *TypeParam {
 	// Always increment lastID, even if it is not used.
 	id := nextID()
@@ -50,9 +51,7 @@ func (check *Checker) newTypeParam(obj *TypeName, constraint Type) *TypeParam {
 	// iface may mutate typ.bound, so we must ensure that iface() is called
 	// at least once before the resulting TypeParam escapes.
 	if check != nil {
-		check.later(func() {
-			typ.iface()
-		})
+		check.needsCleanup(typ)
 	} else if constraint != nil {
 		typ.iface()
 	}
@@ -93,9 +92,12 @@ func (t *TypeParam) String() string { return TypeString(t, nil) }
 // ----------------------------------------------------------------------------
 // Implementation
 
+func (t *TypeParam) cleanup() {
+	t.iface()
+	t.check = nil
+}
+
 // iface returns the constraint interface of t.
-// TODO(gri) If we make tparamIsIface the default, this should be renamed to under
-//           (similar to Named.under).
 func (t *TypeParam) iface() *Interface {
 	bound := t.bound
 

src/cmd/compile/internal/types2/typexpr.go

@@ -342,7 +342,7 @@ func (check *Checker) typInternal(e0 syntax.Expr, def *Named) (T Type) {
 		return typ
 
 	case *syntax.InterfaceType:
-		typ := new(Interface)
+		typ := check.newInterface()
 		def.setUnderlying(typ)
 		if def != nil {
 			typ.obj = def.obj

src/go/types/check.go

@@ -133,7 +133,7 @@ type Checker struct {
 	untyped  map[ast.Expr]exprInfo // map of expressions without final type
 	delayed  []action              // 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.
+	cleaners []cleaner             // list of types that may need a final cleanup at the end of type-checking
 
 	// environment within which the current object is type-checked (valid only
 	// for the duration of type-checking a specific object)
@@ -212,6 +212,16 @@ func (check *Checker) pop() Object {
 	return obj
 }
 
+type cleaner interface {
+	cleanup()
+}
+
+// needsCleanup records objects/types that implement the cleanup method
+// which will be called at the end of type-checking.
+func (check *Checker) needsCleanup(c cleaner) {
+	check.cleaners = append(check.cleaners, c)
+}
+
 // NewChecker returns a new Checker instance for a given package.
 // Package files may be added incrementally via checker.Files.
 func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Checker {
@@ -255,6 +265,8 @@ func (check *Checker) initFiles(files []*ast.File) {
 	check.methods = nil
 	check.untyped = nil
 	check.delayed = nil
+	check.objPath = nil
+	check.cleaners = nil
 
 	// determine package name and collect valid files
 	pkg := check.pkg
@@ -304,22 +316,37 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 
 	defer check.handleBailout(&err)
 
+	print := func(msg string) {
+		if trace {
+			fmt.Println()
+			fmt.Println(msg)
+		}
+	}
+
+	print("== initFiles ==")
 	check.initFiles(files)
 
+	print("== collectObjects ==")
 	check.collectObjects()
 
+	print("== packageObjects ==")
 	check.packageObjects()
 
+	print("== processDelayed ==")
 	check.processDelayed(0) // incl. all functions
 
-	check.expandDefTypes()
+	print("== cleanup ==")
+	check.cleanup()
 
+	print("== initOrder ==")
 	check.initOrder()
 
 	if !check.conf.DisableUnusedImportCheck {
+		print("== unusedImports ==")
 		check.unusedImports()
 	}
 
+	print("== recordUntyped ==")
 	check.recordUntyped()
 
 	if check.firstErr == nil {
@@ -337,7 +364,6 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 	check.recvTParamMap = nil
 	check.brokenAliases = nil
 	check.unionTypeSets = nil
-	check.defTypes = nil
 	check.ctxt = nil
 
 	// TODO(rFindley) There's more memory we should release at this point.
@@ -365,27 +391,13 @@ 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
+// cleanup runs cleanup for all collected cleaners.
+func (check *Checker) cleanup() {
+	// Don't use a range clause since Named.cleanup may add more cleaners.
+	for i := 0; i < len(check.cleaners); i++ {
+		check.cleaners[i].cleanup()
 	}
+	check.cleaners = nil
 }
 
 func (check *Checker) record(x *operand) {