Skip to content

Commit 39da9ae

Browse files
committed
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 <[email protected]> Trust: Robert Griesemer <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent bfd7798 commit 39da9ae

File tree

3 files changed

+59
-14
lines changed

3 files changed

+59
-14
lines changed

src/go/types/decl.go

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -577,15 +577,37 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
577577
// n0.check != nil, the cycle is reported.
578578
func (n0 *Named) under() Type {
579579
u := n0.underlying
580-
if u == nil {
581-
return Typ[Invalid]
580+
581+
if u == Typ[Invalid] {
582+
return u
582583
}
583584

584585
// If the underlying type of a defined type is not a defined
585-
// type, then that is the desired underlying type.
586+
// (incl. instance) type, then that is the desired underlying
587+
// type.
588+
switch u.(type) {
589+
case nil:
590+
return Typ[Invalid]
591+
default:
592+
// common case
593+
return u
594+
case *Named, *instance:
595+
// handled below
596+
}
597+
598+
if n0.check == nil {
599+
panic("internal error: Named.check == nil but type is incomplete")
600+
}
601+
602+
// Invariant: after this point n0 as well as any named types in its
603+
// underlying chain should be set up when this function exits.
604+
check := n0.check
605+
606+
// If we can't expand u at this point, it is invalid.
586607
n := asNamed(u)
587608
if n == nil {
588-
return u // common case
609+
n0.underlying = Typ[Invalid]
610+
return n0.underlying
589611
}
590612

591613
// Otherwise, follow the forward chain.
@@ -597,7 +619,16 @@ func (n0 *Named) under() Type {
597619
u = Typ[Invalid]
598620
break
599621
}
600-
n1 := asNamed(u)
622+
var n1 *Named
623+
switch u1 := u.(type) {
624+
case *Named:
625+
n1 = u1
626+
case *instance:
627+
n1, _ = u1.expand().(*Named)
628+
if n1 == nil {
629+
u = Typ[Invalid]
630+
}
631+
}
601632
if n1 == nil {
602633
break // end of chain
603634
}
@@ -608,11 +639,7 @@ func (n0 *Named) under() Type {
608639

609640
if i, ok := seen[n]; ok {
610641
// cycle
611-
// TODO(rFindley) revert this to a method on Checker. Having a possibly
612-
// nil Checker on Named and TypeParam is too subtle.
613-
if n0.check != nil {
614-
n0.check.cycleError(path[i:])
615-
}
642+
check.cycleError(path[i:])
616643
u = Typ[Invalid]
617644
break
618645
}
@@ -622,8 +649,8 @@ func (n0 *Named) under() Type {
622649
// We should never have to update the underlying type of an imported type;
623650
// those underlying types should have been resolved during the import.
624651
// Also, doing so would lead to a race condition (was issue #31749).
625-
// Do this check always, not just in debug more (it's cheap).
626-
if n0.check != nil && n.obj.pkg != n0.check.pkg {
652+
// Do this check always, not just in debug mode (it's cheap).
653+
if n.obj.pkg != check.pkg {
627654
panic("internal error: imported type with unresolved underlying type")
628655
}
629656
n.underlying = u
@@ -665,7 +692,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *Named) {
665692
} else {
666693
// defined type declaration
667694

668-
named := &Named{check: check, obj: obj}
695+
named := check.newNamed(obj, nil, nil)
669696
def.setUnderlying(named)
670697
obj.typ = named // make sure recursive type declarations terminate
671698

src/go/types/sanitize.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ func (s sanitizer) typ(typ Type) Type {
135135
}
136136

137137
case *Named:
138+
if debug && t.check != nil {
139+
panic("internal error: Named.check != nil")
140+
}
138141
if orig := s.typ(t.orig); orig != t.orig {
139142
t.orig = orig
140143
}

src/go/types/type.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ func (c *Chan) Elem() Type { return c.elem }
644644

645645
// A Named represents a named (defined) type.
646646
type Named struct {
647-
check *Checker // for Named.under implementation
647+
check *Checker // for Named.under implementation; nilled once under has been called
648648
info typeInfo // for cycle detection
649649
obj *TypeName // corresponding declared object
650650
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)
673673
if obj.typ == nil {
674674
obj.typ = typ
675675
}
676+
// Ensure that typ is always expanded, at which point the check field can be
677+
// nilled out.
678+
//
679+
// Note that currently we cannot nil out check inside typ.under(), because
680+
// it's possible that typ is expanded multiple times.
681+
//
682+
// TODO(rFindley): clean this up so that under is the only function mutating
683+
// named types.
684+
check.later(func() {
685+
switch typ.under().(type) {
686+
case *Named, *instance:
687+
panic("internal error: unexpanded underlying type")
688+
}
689+
typ.check = nil
690+
})
676691
return typ
677692
}
678693

0 commit comments

Comments
 (0)