[dev.typeparams] go/types: backport lazy loading changes from CL 336252

When CL 336252 was created (itself a port of CL 335929), types2
tests revealed that lazy expansion of instances was not behaving
correctly with respect to lazy loading of Named types.

This CL ports the fixes from CL 336252 back to go/types.

Change-Id: Iffc6c84a708449633153b800dfb98ff57402893c
Reviewed-on: https://go-review.googlesource.com/c/go/+/338369
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>

9 files changed, 61 insertions, 43 deletions

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index ad88c30282..831b1da589 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -317,7 +317,7 @@ func (check *Checker) validType(typ Type, path []Object) typeInfo {
 		}
 
 	case *Named:
-		t.complete()
+		t.expand()
 		// don't touch the type if it is from a different package or the Universe scope
 		// (doing so would lead to a race condition - was issue #35049)
 		if t.obj.pkg != check.pkg {
@@ -747,7 +747,7 @@ func (check *Checker) collectMethods(obj *TypeName) {
 		}
 
 		if base != nil {
-			base.expand() // TODO(mdempsky): Probably unnecessary.
+			base.load() // TODO(mdempsky): Probably unnecessary.
 			base.methods = append(base.methods, m)
 		}
 	}
diff --git a/src/go/types/instance.go b/src/go/types/instance.go
index 5e0447b434..1223c9f6f1 100644
--- a/src/go/types/instance.go
+++ b/src/go/types/instance.go
@@ -8,34 +8,37 @@ package types
 
 import "go/token"
 
-// instance holds a Checker along with syntactic information
-// information, for use in lazy instantiation.
+// instance holds position information for use in lazy instantiation.
+//
+// TODO(rfindley): come up with a better name for this type, now that its usage
+// has changed.
 type instance struct {
-	check   *Checker
 	pos     token.Pos   // position of type instantiation; for error reporting only
 	posList []token.Pos // position of each targ; for error reporting only
 }
 
-// complete ensures that the underlying type of n is instantiated.
+// expand ensures that the underlying type of n is instantiated.
 // The underlying type will be Typ[Invalid] if there was an error.
 // TODO(rfindley): expand would be a better name for this method, but conflicts
 // with the existing concept of lazy expansion. Need to reconcile this.
-func (n *Named) complete() {
-	if n.instance != nil && len(n.targs) > 0 && n.underlying == nil {
-		check := n.instance.check
-		inst := check.instantiate(n.instance.pos, n.orig.underlying, n.TParams().list(), n.targs, n.instance.posList)
+func (n *Named) expand() {
+	if n.instance != nil {
+		// n must be loaded before instantiation, in order to have accurate
+		// tparams. This is done implicitly by the call to n.TParams, but making it
+		// explicit is harmless: load is idempotent.
+		n.load()
+		inst := n.check.instantiate(n.instance.pos, n.orig.underlying, n.TParams().list(), n.targs, n.instance.posList)
 		n.underlying = inst
 		n.fromRHS = inst
-		n.methods = n.orig.methods
+		n.instance = nil
 	}
 }
 
-// expand expands a type instance into its instantiated
-// type and leaves all other types alone. expand does
-// not recurse.
+// expand expands uninstantiated named types and leaves all other types alone.
+// expand does not recurse.
 func expand(typ Type) Type {
 	if t, _ := typ.(*Named); t != nil {
-		t.complete()
+		t.expand()
 	}
 	return typ
 }
diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go
index 1d3bbc2667..28d68cad0e 100644
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -105,7 +105,9 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, tparams []*TypeName,
 // instantiating the type until needed. typ must be a *Named
 // type.
 func (check *Checker) InstantiateLazy(pos token.Pos, typ Type, targs []Type, posList []token.Pos, verify bool) Type {
-	base := asNamed(typ)
+	// Don't use asNamed here: we don't want to expand the base during lazy
+	// instantiation.
+	base := typ.(*Named)
 	if base == nil {
 		panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ))
 	}
@@ -116,15 +118,18 @@ func (check *Checker) InstantiateLazy(pos token.Pos, typ Type, targs []Type, pos
 	}
 	h := instantiatedHash(base, targs)
 	if check != nil {
+		// typ may already have been instantiated with identical type arguments. In
+		// that case, re-use the existing instance.
 		if named := check.typMap[h]; named != nil {
 			return named
 		}
 	}
 
 	tname := NewTypeName(pos, base.obj.pkg, base.obj.name, nil)
-	named := check.newNamed(tname, base, nil, base.TParams(), base.methods) // methods are instantiated lazily
+	named := check.newNamed(tname, base, nil, nil, nil) // methods and tparams are set when named is loaded.
 	named.targs = targs
-	named.instance = &instance{check, pos, posList}
+	named.instance = &instance{pos, posList}
+
 	if check != nil {
 		check.typMap[h] = named
 	}
diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go
index 8b1d70a978..07baf2a48b 100644
--- a/src/go/types/lookup.go
+++ b/src/go/types/lookup.go
@@ -121,7 +121,7 @@ func lookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string) (o
 				seen[named] = true
 
 				// look for a matching attached method
-				named.expand()
+				named.load()
 				if i, m := lookupMethod(named.methods, pkg, name); m != nil {
 					// potential match
 					// caution: method may not have a proper signature yet
diff --git a/src/go/types/named.go b/src/go/types/named.go
index 03af3fbc5a..87eaa3179e 100644
--- a/src/go/types/named.go
+++ b/src/go/types/named.go
@@ -10,12 +10,13 @@ import "sync"
 
 // A Named represents a named (defined) type.
 type Named struct {
-	instance   *instance   // syntactic information for lazy instantiation
+	check      *Checker
 	info       typeInfo    // for cycle detection
 	obj        *TypeName   // corresponding declared object
 	orig       *Named      // original, uninstantiated type
 	fromRHS    Type        // type (on RHS of declaration) this *Named type is derived of (for cycle reporting)
 	underlying Type        // possibly a *Named during setup; never a *Named once set up completely
+	instance   *instance   // syntactic information for lazy instantiation
 	tparams    *TypeParams // type parameters, or nil
 	targs      []Type      // type arguments (after instantiation), or nil
 	methods    []*Func     // methods declared for this type (not the method set of this type); signatures are type-checked lazily
@@ -34,7 +35,19 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 	return (*Checker)(nil).newNamed(obj, nil, underlying, nil, methods)
 }
 
-func (t *Named) expand() *Named {
+func (t *Named) load() *Named {
+	// If t is an instantiated type, it derives its methods and tparams from its
+	// base type. Since we expect type parameters and methods to be set after a
+	// call to load, we must load the base and copy here.
+	//
+	// underlying is set when t is expanded.
+	//
+	// By convention, a type instance is loaded iff its tparams are set.
+	if len(t.targs) > 0 && t.tparams == nil {
+		t.orig.load()
+		t.tparams = t.orig.tparams
+		t.methods = t.orig.methods
+	}
 	if t.resolve == nil {
 		return t
 	}
@@ -65,13 +78,7 @@ func (t *Named) expand() *Named {
 
 // newNamed is like NewNamed but with a *Checker receiver and additional orig argument.
 func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tparams *TypeParams, methods []*Func) *Named {
-	var inst *instance
-	if check != nil {
-		inst = &instance{
-			check: check,
-		}
-	}
-	typ := &Named{instance: inst, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, tparams: tparams, methods: methods}
+	typ := &Named{check: check, obj: obj, orig: orig, fromRHS: underlying, underlying: underlying, tparams: tparams, methods: methods}
 	if typ.orig == nil {
 		typ.orig = typ
 	}
@@ -92,7 +99,7 @@ func (check *Checker) newNamed(obj *TypeName, orig *Named, underlying Type, tpar
 			case *Named:
 				panic("internal error: unexpanded underlying type")
 			}
-			typ.instance = nil
+			typ.check = nil
 		})
 	}
 	return typ
@@ -110,10 +117,10 @@ func (t *Named) _Orig() *Named { return t.orig }
 
 // TParams returns the type parameters of the named type t, or nil.
 // The result is non-nil for an (originally) parameterized type even if it is instantiated.
-func (t *Named) TParams() *TypeParams { return t.expand().tparams }
+func (t *Named) TParams() *TypeParams { return t.load().tparams }
 
 // SetTParams sets the type parameters of the named type t.
-func (t *Named) SetTParams(tparams []*TypeName) { t.expand().tparams = bindTParams(tparams) }
+func (t *Named) SetTParams(tparams []*TypeName) { t.load().tparams = bindTParams(tparams) }
 
 // TArgs returns the type arguments after instantiation of the named type t, or nil if not instantiated.
 func (t *Named) TArgs() []Type { return t.targs }
@@ -122,10 +129,10 @@ func (t *Named) TArgs() []Type { return t.targs }
 func (t *Named) SetTArgs(args []Type) { t.targs = args }
 
 // NumMethods returns the number of explicit methods whose receiver is named type t.
-func (t *Named) NumMethods() int { return len(t.expand().methods) }
+func (t *Named) NumMethods() int { return len(t.load().methods) }
 
 // Method returns the i'th method of named type t for 0 <= i < t.NumMethods().
-func (t *Named) Method(i int) *Func { return t.expand().methods[i] }
+func (t *Named) Method(i int) *Func { return t.load().methods[i] }
 
 // SetUnderlying sets the underlying type and marks t as complete.
 func (t *Named) SetUnderlying(underlying Type) {
@@ -135,18 +142,18 @@ func (t *Named) SetUnderlying(underlying Type) {
 	if _, ok := underlying.(*Named); ok {
 		panic("types.Named.SetUnderlying: underlying type must not be *Named")
 	}
-	t.expand().underlying = underlying
+	t.load().underlying = underlying
 }
 
 // AddMethod adds method m unless it is already in the method list.
 func (t *Named) AddMethod(m *Func) {
-	t.expand()
+	t.load()
 	if i, _ := lookupMethod(t.methods, m.pkg, m.name); i < 0 {
 		t.methods = append(t.methods, m)
 	}
 }
 
-func (t *Named) Underlying() Type { return t.expand().underlying }
+func (t *Named) Underlying() Type { return t.load().underlying }
 func (t *Named) String() string   { return TypeString(t, nil) }
 
 // ----------------------------------------------------------------------------
@@ -159,7 +166,7 @@ func (t *Named) String() string   { return TypeString(t, nil) }
 // is detected, the result is Typ[Invalid]. If a cycle is detected and
 // n0.check != nil, the cycle is reported.
 func (n0 *Named) under() Type {
-	n0.complete()
+	n0.expand()
 
 	u := n0.Underlying()
 
@@ -180,13 +187,13 @@ func (n0 *Named) under() Type {
 		// handled below
 	}
 
-	if n0.instance == nil || n0.instance.check == nil {
+	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.instance.check
+	check := n0.check
 
 	// If we can't expand u at this point, it is invalid.
 	n := asNamed(u)
@@ -207,7 +214,7 @@ func (n0 *Named) under() Type {
 		var n1 *Named
 		switch u1 := u.(type) {
 		case *Named:
-			u1.complete()
+			u1.expand()
 			n1 = u1
 		}
 		if n1 == nil {
diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go
index 181e2fcfc5..41e0c25d6b 100644
--- a/src/go/types/predicates.go
+++ b/src/go/types/predicates.go
@@ -21,7 +21,7 @@ func isNamed(typ Type) bool {
 func isGeneric(typ Type) bool {
 	// A parameterized type is only instantiated if it doesn't have an instantiation already.
 	named, _ := typ.(*Named)
-	return named != nil && named.obj != nil && named.TParams() != nil && named.targs == nil
+	return named != nil && named.obj != nil && named.targs == nil && named.TParams() != nil
 }
 
 func is(typ Type, what BasicInfo) bool {
diff --git a/src/go/types/sizeof_test.go b/src/go/types/sizeof_test.go
index 29e298103b..c8758663ec 100644
--- a/src/go/types/sizeof_test.go
+++ b/src/go/types/sizeof_test.go
@@ -30,7 +30,7 @@ func TestSizeof(t *testing.T) {
 		{Interface{}, 40, 80},
 		{Map{}, 16, 32},
 		{Chan{}, 12, 24},
-		{Named{}, 76, 144},
+		{Named{}, 80, 152},
 		{TypeParam{}, 28, 48},
 		{top{}, 0, 0},
 
diff --git a/src/go/types/subst.go b/src/go/types/subst.go
index 60fc7ae819..c05e51d425 100644
--- a/src/go/types/subst.go
+++ b/src/go/types/subst.go
@@ -244,7 +244,7 @@ func (subst *subster) typ(typ Type) Type {
 		named := subst.check.newNamed(tname, t, t.Underlying(), t.TParams(), t.methods) // method signatures are updated lazily
 		named.targs = newTargs
 		subst.typMap[h] = named
-		t.complete() // must happen after typMap update to avoid infinite recursion
+		t.expand() // must happen after typMap update to avoid infinite recursion
 
 		// do the substitution
 		dump(">>> subst %s with %s (new: %s)", t.underlying, subst.smap, newTargs)
diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go
index 6a9e7f2ac8..74b18a9ec8 100644
--- a/src/go/types/typestring.go
+++ b/src/go/types/typestring.go
@@ -270,6 +270,9 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) {
 		}
 
 	case *Named:
+		if t.instance != nil {
+			buf.WriteByte(instanceMarker)
+		}
 		writeTypeName(buf, t.obj, qf)
 		if t.targs != nil {
 			// instantiated type