cmd/compile/internal/types2: clean up panics in instantiation

Clean up a few issues related to panicking during invalid instantiation.
 - Panic early in instantiateLazy when check == nil and verify == true.
   Otherwise, we would panic at check.later.
 - Always panic when check == nil and verify == true, even if targs is
   of incorrect length. This is more consistent behavior.
 - Lift the check for len(posList) <= len(targs) out of
   Checker.instantiate. This is the only reason why posList is passed to
   that function, and doing this allows us to eliminate posList from
   instance. At this point instance is close to being unnecessary, so
   update a TODO to this effect.

Change-Id: Id5f44cbb1a5897aef10ce2a573aa78acd7ae4026
Reviewed-on: https://go-review.googlesource.com/c/go/+/341862
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>
This commit is contained in:
Robert Findley 2021-08-13 12:03:29 -04:00
parent 4a0fd73ead
commit 165ebd85a7
2 changed files with 27 additions and 22 deletions

View File

@ -54,15 +54,18 @@ func (check *Checker) Instantiate(pos syntax.Pos, typ Type, targs []Type, posLis
// only types and functions can be generic // only types and functions can be generic
panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ)) panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ))
} }
inst := check.instantiate(pos, typ, tparams, targs, nil)
inst := check.instantiate(pos, typ, tparams, targs, posList, nil) if verify {
if verify && len(tparams) == len(targs) { assert(len(posList) <= len(targs))
check.verify(pos, tparams, targs, posList) if len(tparams) == len(targs) {
check.verify(pos, tparams, targs, posList)
}
} }
return inst return inst
} }
func (check *Checker) instantiate(pos syntax.Pos, typ Type, tparams []*TypeName, targs []Type, posList []syntax.Pos, typMap map[string]*Named) (res Type) { func (check *Checker) instantiate(pos syntax.Pos, typ Type, tparams []*TypeName, targs []Type, typMap map[string]*Named) (res Type) {
// the number of supplied types must match the number of type parameters // the number of supplied types must match the number of type parameters
if len(targs) != len(tparams) { if len(targs) != len(tparams) {
// TODO(gri) provide better error message // TODO(gri) provide better error message
@ -89,8 +92,6 @@ func (check *Checker) instantiate(pos syntax.Pos, typ Type, tparams []*TypeName,
}() }()
} }
assert(len(posList) <= len(targs))
if len(tparams) == 0 { if len(tparams) == 0 {
return typ // nothing to do (minor optimization) return typ // nothing to do (minor optimization)
} }
@ -100,15 +101,21 @@ func (check *Checker) instantiate(pos syntax.Pos, typ Type, tparams []*TypeName,
// instantiateLazy avoids actually instantiating the type until needed. typ // instantiateLazy avoids actually instantiating the type until needed. typ
// must be a *Named type. // must be a *Named type.
func (check *Checker) instantiateLazy(pos syntax.Pos, base *Named, targs []Type, posList []syntax.Pos, verify bool) Type { func (check *Checker) instantiateLazy(pos syntax.Pos, orig *Named, targs []Type, posList []syntax.Pos, verify bool) Type {
if verify && base.TParams().Len() == len(targs) { if verify {
// TODO: lift the nil check in verify to here. if check == nil {
check.later(func() { // Provide a more useful panic instead of panicking at check.later below.
check.verify(pos, base.tparams.list(), targs, posList) panic("cannot have nil Checker if verifying constraints")
}) }
assert(len(posList) <= len(targs))
if orig.TParams().Len() == len(targs) {
check.later(func() {
check.verify(pos, orig.tparams.list(), targs, posList)
})
}
} }
h := instantiatedHash(base, targs) h := instantiatedHash(orig, targs)
if check != nil { if check != nil {
// typ may already have been instantiated with identical type arguments. In // typ may already have been instantiated with identical type arguments. In
// that case, re-use the existing instance. // that case, re-use the existing instance.
@ -117,10 +124,10 @@ func (check *Checker) instantiateLazy(pos syntax.Pos, base *Named, targs []Type,
} }
} }
tname := NewTypeName(pos, base.obj.pkg, base.obj.name, nil) tname := NewTypeName(pos, orig.obj.pkg, orig.obj.name, nil)
named := check.newNamed(tname, base, nil, nil, nil) // methods and tparams are set when named is loaded named := check.newNamed(tname, orig, nil, nil, nil) // methods and tparams are set when named is loaded
named.targs = targs named.targs = targs
named.instance = &instance{pos, posList} named.instance = &instance{pos}
if check != nil { if check != nil {
check.typMap[h] = named check.typMap[h] = named
} }
@ -132,7 +139,6 @@ func (check *Checker) verify(pos syntax.Pos, tparams []*TypeName, targs []Type,
if check == nil { if check == nil {
panic("cannot have nil Checker if verifying constraints") panic("cannot have nil Checker if verifying constraints")
} }
smap := makeSubstMap(tparams, targs) smap := makeSubstMap(tparams, targs)
for i, tname := range tparams { for i, tname := range tparams {
// best position for error reporting // best position for error reporting

View File

@ -244,11 +244,10 @@ func (n *Named) setUnderlying(typ Type) {
// instance holds position 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 // TODO(rfindley): instance is probably unnecessary now. See if it can be
// has changed. // eliminated.
type instance struct { type instance struct {
pos syntax.Pos // position of type instantiation; for error reporting only pos syntax.Pos // position of type instantiation; for error reporting only
posList []syntax.Pos // position of each targ; for error reporting only
} }
// expand ensures that the underlying type of n is instantiated. // expand ensures that the underlying type of n is instantiated.
@ -272,7 +271,7 @@ func (n *Named) expand(typMap map[string]*Named) *Named {
} }
} }
inst := n.check.instantiate(n.instance.pos, n.orig.underlying, n.TParams().list(), n.targs, n.instance.posList, typMap) inst := n.check.instantiate(n.instance.pos, n.orig.underlying, n.TParams().list(), n.targs, typMap)
n.underlying = inst n.underlying = inst
n.fromRHS = inst n.fromRHS = inst
n.instance = nil n.instance = nil