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>

2 files changed, 27 insertions, 22 deletions

diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go
index fff2635456..d2383db35c 100644
--- a/src/cmd/compile/internal/types2/instantiate.go
+++ b/src/cmd/compile/internal/types2/instantiate.go
@@ -54,15 +54,18 @@ func (check *Checker) Instantiate(pos syntax.Pos, typ Type, targs []Type, posLis
 		// only types and functions can be generic
 		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 && len(tparams) == len(targs) {
-		check.verify(pos, tparams, targs, posList)
+	if verify {
+		assert(len(posList) <= len(targs))
+		if len(tparams) == len(targs) {
+			check.verify(pos, tparams, targs, posList)
+		}
 	}
 	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
 	if len(targs) != len(tparams) {
 		// 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 {
 		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
 // must be a *Named type.
-func (check *Checker) instantiateLazy(pos syntax.Pos, base *Named, targs []Type, posList []syntax.Pos, verify bool) Type {
-	if verify && base.TParams().Len() == len(targs) {
-		// TODO: lift the nil check in verify to here.
-		check.later(func() {
-			check.verify(pos, base.tparams.list(), targs, posList)
-		})
+func (check *Checker) instantiateLazy(pos syntax.Pos, orig *Named, targs []Type, posList []syntax.Pos, verify bool) Type {
+	if verify {
+		if check == nil {
+			// Provide a more useful panic instead of panicking at check.later below.
+			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 {
 		// typ may already have been instantiated with identical type arguments. In
 		// 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)
-	named := check.newNamed(tname, base, nil, nil, nil) // methods and tparams are set when named is loaded
+	tname := NewTypeName(pos, orig.obj.pkg, orig.obj.name, nil)
+	named := check.newNamed(tname, orig, nil, nil, nil) // methods and tparams are set when named is loaded
 	named.targs = targs
-	named.instance = &instance{pos, posList}
+	named.instance = &instance{pos}
 	if check != nil {
 		check.typMap[h] = named
 	}
@@ -132,7 +139,6 @@ func (check *Checker) verify(pos syntax.Pos, tparams []*TypeName, targs []Type,
 	if check == nil {
 		panic("cannot have nil Checker if verifying constraints")
 	}
-
 	smap := makeSubstMap(tparams, targs)
 	for i, tname := range tparams {
 		// best position for error reporting
diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 3ce9c5b0c7..97239414f6 100644
--- a/src/cmd/compile/internal/types2/named.go
+++ b/src/cmd/compile/internal/types2/named.go
@@ -244,11 +244,10 @@ func (n *Named) setUnderlying(typ Type) {
 
 // instance holds position information for use in lazy instantiation.
 //
-// TODO(rfindley): come up with a better name for this type, now that its usage
-// has changed.
+// TODO(rfindley): instance is probably unnecessary now. See if it can be
+// eliminated.
 type instance struct {
-	pos     syntax.Pos   // position of type instantiation; for error reporting only
-	posList []syntax.Pos // position of each targ; for error reporting only
+	pos syntax.Pos // position of type instantiation; for error reporting only
 }
 
 // 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.fromRHS = inst
 		n.instance = nil