From cb4e1de0213c836983d1b441386c53e1a66e1b0a Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Sat, 11 Sep 2021 23:06:41 -0400 Subject: [PATCH] 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 Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/types/named.go | 38 ++++++++++++++++++++++---------------- src/go/types/subst.go | 31 +++++++------------------------ 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/go/types/named.go b/src/go/types/named.go index 66ae0123798..00fde16445c 100644 --- 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 3491541dcb3..16aafd622e9 100644 --- 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 - } + subst := subster{ + pos: pos, + smap: smap, + check: check, + env: check.bestEnv(env), } - 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.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)