cmd/compile/internal/types2: minor cleanup of instantiation

This is a clean port of CL 349429 from go/types to types2
with minor adjustments for types2 names.

Change-Id: Ie6a39a01f074acb9e6565ffacb34c94666ae9a95
Reviewed-on: https://go-review.googlesource.com/c/go/+/349999
Trust: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: griesemer

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

@@ -219,6 +219,21 @@ func (n *Named) setUnderlying(typ Type) {
 	}
 }
 
+// bestEnv returns the best available environment. In order of preference:
+// - the given env, if non-nil
+// - the Checker env, if check is non-nil
+// - a new environment
+func (check *Checker) bestEnv(env *Environment) *Environment {
+	if env != nil {
+		return env
+	}
+	if check != nil {
+		assert(check.conf.Environment != nil)
+		return check.conf.Environment
+	}
+	return NewEnvironment()
+}
+
 // expandNamed ensures that the underlying type of n is instantiated.
 // The underlying type will be Typ[Invalid] if there was an error.
 func expandNamed(env *Environment, n *Named, instPos syntax.Pos) (tparams *TypeParamList, underlying Type, methods []*Func) {
@@ -227,24 +242,15 @@ func expandNamed(env *Environment, n *Named, instPos syntax.Pos) (tparams *TypeP
 	check := n.check
 
 	if check.validateTArgLen(instPos, n.orig.tparams.Len(), n.targs.Len()) {
-		// TODO(rfindley): handling an optional Checker and Environment here (and
-		// in subst) feels overly complicated. Can we simplify?
-		if env == nil {
-			if check != nil {
-				env = check.conf.Environment
-			} else {
-				// If we're instantiating lazily, we might be outside the scope of a
-				// type-checking pass. In that case we won't have a pre-existing
-				// environment, but don't want to create a duplicate of the current
-				// instance in the process of expansion.
-				env = NewEnvironment()
-			}
-			h := env.TypeHash(n.orig, n.targs.list())
-			// ensure that an instance is recorded for h to avoid infinite recursion.
-			env.typeForHash(h, n)
-		}
+		// We must always have an env, to avoid infinite recursion.
+		env = check.bestEnv(env)
+		h := env.TypeHash(n.orig, n.targs.list())
+		// ensure that an instance is recorded for h to avoid infinite recursion.
+		env.typeForHash(h, n)
+
 		smap := makeSubstMap(n.orig.tparams.list(), n.targs.list())
 		underlying = n.check.subst(instPos, n.orig.underlying, smap, env)
+
 		for i := 0; i < n.orig.NumMethods(); i++ {
 			origm := n.orig.Method(i)
 

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

@@ -52,25 +52,12 @@ func (check *Checker) subst(pos syntax.Pos, typ Type, smap substMap, env *Enviro
 	}
 
 	// general case
-	var subst subster
-	subst.pos = pos
-	subst.smap = smap
-
-	if check != nil {
-		subst.check = check
-		if env == nil {
-			env = check.conf.Environment
-		}
-	}
-	if env == nil {
-		// If we don't have a *Checker and its global type map,
-		// use a local version. Besides avoiding duplicate work,
-		// the type map prevents infinite recursive substitution
-		// for recursive types (example: type T[P any] *T[P]).
-		env = NewEnvironment()
+	subst := subster{
+		pos:   pos,
+		smap:  smap,
+		check: check,
+		env:   check.bestEnv(env),
 	}
-	subst.env = env
-
 	return subst.typ(typ)
 }
 
@@ -227,20 +214,15 @@ func (subst *subster) typ(typ Type) Type {
 		// recursion. The position used here is irrelevant because validation only
 		// occurs on t (we don't call validType on named), but we use subst.pos to
 		// help with debugging.
-		named := subst.check.instance(subst.pos, t.orig, newTArgs, subst.env).(*Named)
-		// TODO(rfindley): we probably don't need to resolve here. Investigate if
-		// this can be removed.
-		named.resolve(subst.env)
-		assert(named.underlying != nil)
+		t.orig.resolve(subst.env)
+		return subst.check.instance(subst.pos, t.orig, newTArgs, subst.env)
 
 		// Note that if we were to expose substitution more generally (not just in
 		// the context of a declaration), we'd have to substitute in
 		// named.underlying as well.
 		//
 		// But this is unnecessary for now.
 
-		return named
-
 	case *TypeParam:
 		return subst.smap.lookup(t)