go/types: ensure that Named.check is nilled out once it is expanded

To support lazy expansion of defined types, *Named holds on to a
*Checker field, which can pin the *Checker in memory. This can have
meaningful memory implications for applications that keep type
information around.

Ensure that the Checker field is nilled out for any Named types that are
instantiated during the type checking pass, by deferring a clean up to
'later' boundaries.

In testing this almost exactly offset the ~6% memory footprint increase
I observed with 1.17.

Fixes #45580

Change-Id: I8aa5bb777573a924afe36e79fa65f8729336bceb
Reviewed-on: https://go-review.googlesource.com/c/go/+/318849
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index 5f38a346cee82600da867b1f968026f0bf196fad..9211febc6da1620a43e9867af2f37c149a6f0753 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -577,15 +577,37 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
 // n0.check != nil, the cycle is reported.
 func (n0 *Named) under() Type {
        u := n0.underlying
-       if u == nil {
-               return Typ[Invalid]
+
+       if u == Typ[Invalid] {
+               return u
        }
 
        // If the underlying type of a defined type is not a defined
-       // type, then that is the desired underlying type.
+       // (incl. instance) type, then that is the desired underlying
+       // type.
+       switch u.(type) {
+       case nil:
+               return Typ[Invalid]
+       default:
+               // common case
+               return u
+       case *Named, *instance:
+               // handled below
+       }
+
+       if n0.check == nil {
+               panic("internal error: Named.check == nil but type is incomplete")
+       }
+
+       // Invariant: after this point n0 as well as any named types in its
+       // underlying chain should be set up when this function exits.
+       check := n0.check
+
+       // If we can't expand u at this point, it is invalid.
        n := asNamed(u)
        if n == nil {
-               return u // common case
+               n0.underlying = Typ[Invalid]
+               return n0.underlying
        }
 
        // Otherwise, follow the forward chain.
@@ -597,7 +619,16 @@ func (n0 *Named) under() Type {
                        u = Typ[Invalid]
                        break
                }
-               n1 := asNamed(u)
+               var n1 *Named
+               switch u1 := u.(type) {
+               case *Named:
+                       n1 = u1
+               case *instance:
+                       n1, _ = u1.expand().(*Named)
+                       if n1 == nil {
+                               u = Typ[Invalid]
+                       }
+               }
                if n1 == nil {
                        break // end of chain
                }
@@ -608,11 +639,7 @@ func (n0 *Named) under() Type {
 
                if i, ok := seen[n]; ok {
                        // cycle
-                       // TODO(rFindley) revert this to a method on Checker. Having a possibly
-                       // nil Checker on Named and TypeParam is too subtle.
-                       if n0.check != nil {
-                               n0.check.cycleError(path[i:])
-                       }
+                       check.cycleError(path[i:])
                        u = Typ[Invalid]
                        break
                }
@@ -622,8 +649,8 @@ func (n0 *Named) under() Type {
                // We should never have to update the underlying type of an imported type;
                // those underlying types should have been resolved during the import.
                // Also, doing so would lead to a race condition (was issue #31749).
-               // Do this check always, not just in debug more (it's cheap).
-               if n0.check != nil && n.obj.pkg != n0.check.pkg {
+               // Do this check always, not just in debug mode (it's cheap).
+               if n.obj.pkg != check.pkg {
                        panic("internal error: imported type with unresolved underlying type")
                }
                n.underlying = u
@@ -665,7 +692,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *Named) {
        } else {
                // defined type declaration
 
-               named := &Named{check: check, obj: obj}
+               named := check.newNamed(obj, nil, nil)
                def.setUnderlying(named)
                obj.typ = named // make sure recursive type declarations terminate
 

diff --git a/src/go/types/sanitize.go b/src/go/types/sanitize.go
index 5970ab38c712dacd6df3b22e770a14ed5e0c48b4..727ec173eac81a4077be56e5f1a1affe9372e88b 100644 (file)
--- a/src/go/types/sanitize.go
+++ b/src/go/types/sanitize.go
@@ -135,6 +135,9 @@ func (s sanitizer) typ(typ Type) Type {
                }
 
        case *Named:
+               if debug && t.check != nil {
+                       panic("internal error: Named.check != nil")
+               }
                if orig := s.typ(t.orig); orig != t.orig {
                        t.orig = orig
                }

diff --git a/src/go/types/type.go b/src/go/types/type.go
index 3303cfc077424f64fc68e6809810780a461d809a..3fdb2365a03ecac28a926b1bcd7ab66a8a319bc5 100644 (file)
--- a/src/go/types/type.go
+++ b/src/go/types/type.go
@@ -644,7 +644,7 @@ func (c *Chan) Elem() Type { return c.elem }
 
 // A Named represents a named (defined) type.
 type Named struct {
-       check      *Checker    // for Named.under implementation
+       check      *Checker    // for Named.under implementation; nilled once under has been called
        info       typeInfo    // for cycle detection
        obj        *TypeName   // corresponding declared object
        orig       Type        // type (on RHS of declaration) this *Named type is derived of (for cycle reporting)
@@ -673,6 +673,21 @@ func (check *Checker) newNamed(obj *TypeName, underlying Type, methods []*Func)
        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.
+       check.later(func() {
+               switch typ.under().(type) {
+               case *Named, *instance:
+                       panic("internal error: unexpanded underlying type")
+               }
+               typ.check = nil
+       })
        return typ
 }