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 <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 7fc84004e3c11dc0c6448293e2cfed92f1b09764..bc4ac87b9dbdd0c56d0e21ed5bf0e0229b755372 100644 (file)
--- a/src/cmd/compile/internal/types2/named.go
+++ b/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)
 

diff --git a/src/cmd/compile/internal/types2/subst.go b/src/cmd/compile/internal/types2/subst.go
index dcff1f822ccef42961ac02a307eed7b61d5d4baa..ee68f226531286d0181365aca8b92e8e53a92f4a 100644 (file)
--- a/src/cmd/compile/internal/types2/subst.go
+++ b/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,11 +214,8 @@ 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
@@ -239,8 +223,6 @@ func (subst *subster) typ(typ Type) Type {
                //
                // But this is unnecessary for now.
 
-               return named
-
        case *TypeParam:
                return subst.smap.lookup(t)