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>

diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go
index fff263545600412727e7063abab69fbecd11f3f7..d2383db35c5680e9740fbecf1b7d282cb5d4329d 100644 (file)
--- 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 3ce9c5b0c79c580e0b76eb841772326ab13724ef..97239414f634e53979160ff966aedc68d126d98e 100644 (file)
--- 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