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 <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Robert Griesemer 2022-02-22 13:06:52 -08:00
parent 163da6feb5
commit e94f7df957
12 changed files with 168 additions and 93 deletions

View file

@ -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) {

View file

@ -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)
}

View file

@ -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

View file

@ -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.

View file

@ -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

View file

@ -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

View file

@ -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) {

View file

@ -56,7 +56,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)
@ -73,6 +73,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
@ -141,6 +150,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 *ast.InterfaceType, def *Named) {
addEmbedded := func(pos token.Pos, typ Type) {
ityp.embeddeds = append(ityp.embeddeds, typ)
@ -210,16 +224,10 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
sortMethods(ityp.methods)
// (don't sort embeddeds: they must correspond to *embedPos entries)
// 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)
}

View file

@ -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 {
@ -362,11 +382,11 @@ func expandNamed(ctxt *Context, n *Named, instPos token.Pos) (tparams *TypeParam
// 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

View file

@ -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.

View file

@ -35,6 +35,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()
@ -49,9 +50,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()
}
@ -95,9 +94,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

View file

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