go/types: minor cleanup of instantiation

This CL addresses a couple TODOs related to instantiation:
 - factor out resolving the best environment
 - don't eagerly resolve substituted instances

Change-Id: I4a5de7ea7939b6f272991071f591d622dec04b53
Reviewed-on: https://go-review.googlesource.com/c/go/+/349429
Trust: Robert Findley <rfindley@google.com>
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/named.go b/src/go/types/named.go
index 66ae0123798ffa0f9e3b230ff2c719d776eec499..00fde16445c1fc7412083bfac3238290abde2d69 100644 (file)
--- a/src/go/types/named.go
+++ b/src/go/types/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 token.Pos) (tparams *TypeParamList, underlying Type, methods []*Func) {
@@ -227,24 +242,15 @@ func expandNamed(env *Environment, n *Named, instPos token.Pos) (tparams *TypePa
        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/go/types/subst.go b/src/go/types/subst.go
index 3491541dcb32d630c8ec557080c3051eb9a08045..16aafd622e97c40428eb2cef12b98c0b316c306b 100644 (file)
--- a/src/go/types/subst.go
+++ b/src/go/types/subst.go
@@ -52,24 +52,12 @@ func (check *Checker) subst(pos token.Pos, typ Type, smap substMap, env *Environ
        }
 
        // 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 +215,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 +224,6 @@ func (subst *subster) typ(typ Type) Type {
                //
                // But this is unnecessary for now.
 
-               return named
-
        case *TypeParam:
                return subst.smap.lookup(t)