go/types: return an error from Instantiate

This is a port of CL 342152 to go/types. Additionally, a panic was
removed from interface substitution, which is a fix from CL 333155 that
was previously missed.

A check for a nil Checker was also removed from types2.instantiate,
since check must not be nil in that method.

Change-Id: I4ea6bdccbd50ea2008ee6d870f702bee5cdd5a8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/342671
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>

src/cmd/compile/internal/types2/instantiate.go

@@ -53,7 +53,8 @@ func Instantiate(check *Checker, typ Type, targs []Type, validate bool) (Type, e
 // later in the type checking pass. For Named types the resulting instance will
 // be unexpanded.
 func (check *Checker) instantiate(pos syntax.Pos, typ Type, targs []Type, posList []syntax.Pos) (res Type) {
-	if check != nil && check.conf.Trace {
+	assert(check != nil)
+	if check.conf.Trace {
 		check.trace(pos, "-- instantiating %s with %s", typ, typeListString(targs))
 		check.indent++
 		defer func() {
@@ -69,7 +70,6 @@ func (check *Checker) instantiate(pos syntax.Pos, typ Type, targs []Type, posLis
 		}()
 	}
 
-	assert(check != nil)
 	inst := check.instance(pos, typ, targs)
 
 	assert(len(posList) <= len(targs))

src/go/types/api.go

@@ -62,6 +62,18 @@ func (err Error) Error() string {
 	return fmt.Sprintf("%s: %s", err.Fset.Position(err.Pos), err.Msg)
 }
 
+// An ArgumentError holds an error associated with an argument index.
+type ArgumentError struct {
+	index int
+	error
+}
+
+// Index returns the positional index of the argument associated with the
+// error.
+func (e ArgumentError) Index() int {
+	return e.index
+}
+
 // An Importer resolves import paths to Packages.
 //
 // CAUTION: This interface does not support the import of locally

src/go/types/api_test.go

@@ -1856,8 +1856,10 @@ func TestInstantiate(t *testing.T) {
 
 	// instantiation should succeed (no endless recursion)
 	// even with a nil *Checker
-	var check *Checker
+	res, err := Instantiate(nil, T, []Type{Typ[Int]}, false)
-	res := check.Instantiate(token.NoPos, T, []Type{Typ[Int]}, nil, false)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	// instantiated type should point to itself
 	if p := res.Underlying().(*Pointer).Elem(); p != res {
@@ -1865,6 +1867,39 @@ func TestInstantiate(t *testing.T) {
 	}
 }
 
+func TestInstantiateErrors(t *testing.T) {
+	tests := []struct {
+		src    string // by convention, T must be the type being instantiated
+		targs  []Type
+		wantAt int // -1 indicates no error
+	}{
+		{"type T[P interface{~string}] int", []Type{Typ[Int]}, 0},
+		{"type T[P1 interface{int}, P2 interface{~string}] int", []Type{Typ[Int], Typ[Int]}, 1},
+		{"type T[P1 any, P2 interface{~[]P1}] int", []Type{Typ[Int], NewSlice(Typ[String])}, 1},
+		{"type T[P1 interface{~[]P2}, P2 any] int", []Type{NewSlice(Typ[String]), Typ[Int]}, 0},
+	}
+
+	for _, test := range tests {
+		src := genericPkg + "p; " + test.src
+		pkg, err := pkgFor(".", src, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		T := pkg.Scope().Lookup("T").Type().(*Named)
+
+		_, err = Instantiate(nil, T, test.targs, true)
+		if err == nil {
+			t.Fatalf("Instantiate(%v, %v) returned nil error, want non-nil", T, test.targs)
+		}
+
+		gotAt := err.(ArgumentError).Index()
+		if gotAt != test.wantAt {
+			t.Errorf("Instantate(%v, %v): error at index %d, want index %d", T, test.targs, gotAt, test.wantAt)
+		}
+	}
+}
+
 func TestInstanceIdentity(t *testing.T) {
 	imports := make(testImporter)
 	conf := Config{Importer: imports}

src/go/types/call.go

@@ -60,7 +60,7 @@ func (check *Checker) funcInst(x *operand, ix *typeparams.IndexExpr) {
 	}
 
 	// instantiate function signature
-	res := check.Instantiate(x.Pos(), sig, targs, poslist, true).(*Signature)
+	res := check.instantiate(x.Pos(), sig, targs, poslist).(*Signature)
 	assert(res.TParams().Len() == 0) // signature is not generic anymore
 	if inferred {
 		check.recordInferred(ix.Orig, targs, res)
@@ -332,7 +332,7 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, targs []Type
 		}
 
 		// compute result signature
-		rsig = check.Instantiate(call.Pos(), sig, targs, nil, true).(*Signature)
+		rsig = check.instantiate(call.Pos(), sig, targs, nil).(*Signature)
 		assert(rsig.TParams().Len() == 0) // signature is not generic anymore
 		check.recordInferred(call, targs, rsig)
 

src/go/types/instantiate.go

@@ -13,53 +13,65 @@ import (
 	"go/token"
 )
 
-// Instantiate instantiates the type typ with the given type arguments
+// Instantiate instantiates the type typ with the given type arguments targs.
-// targs. To check type constraint satisfaction, verify must be set.
+// typ must be a *Named or a *Signature type, and its number of type parameters
-// pos and posList correspond to the instantiation and type argument
+// must match the number of provided type arguments. The result is a new,
-// positions respectively; posList may be nil or shorter than the number
+// instantiated (not parameterized) type of the same kind (either a *Named or a
-// of type arguments provided.
+// *Signature). Any methods attached to a *Named are simply copied; they are
-// typ must be a *Named or a *Signature type, and its number of type
+// not instantiated.
-// parameters must match the number of provided type arguments.
+//
-// The receiver (check) may be nil if and only if verify is not set.
+// If check is non-nil, it will be used to de-dupe the instance against
-// The result is a new, instantiated (not generic) type of the same kind
+// previous instances with the same identity.
-// (either a *Named or a *Signature).
+//
-// Any methods attached to a *Named are simply copied; they are not
+// If verify is set and constraint satisfaction fails, the returned error may
-// instantiated.
+// be of dynamic type ArgumentError indicating which type argument did not
-func (check *Checker) Instantiate(pos token.Pos, typ Type, targs []Type, posList []token.Pos, verify bool) (res Type) {
+// satisfy its corresponding type parameter constraint, and why.
-	var inst Type
+//
+// TODO(rfindley): change this function to also return an error if lengths of
+// tparams and targs do not match.
+func Instantiate(check *Checker, typ Type, targs []Type, validate bool) (Type, error) {
+	inst := check.instance(token.NoPos, typ, targs)
+
+	var err error
+	if validate {
+		var tparams []*TypeName
 		switch t := typ.(type) {
 		case *Named:
-		inst = check.instantiateLazy(pos, t, targs)
+			tparams = t.TParams().list()
 		case *Signature:
-		tparams := t.TParams().list()
+			tparams = t.TParams().list()
-		defer func() {
-			// If we had an unexpected failure somewhere don't panic below when
-			// asserting res.(*Signature). Check for *Signature in case Typ[Invalid]
-			// is returned.
-			if _, ok := res.(*Signature); !ok {
-				return
 		}
-			// If the signature doesn't use its type parameters, subst
+		if i, err := check.verify(token.NoPos, tparams, targs); err != nil {
-			// will not make a copy. In that case, make a copy now (so
+			return inst, ArgumentError{i, err}
-			// we can set tparams to nil w/o causing side-effects).
-			if t == res {
-				copy := *t
-				res = &copy
 		}
-			// After instantiating a generic signature, it is not generic
-			// anymore; we need to set tparams to nil.
-			res.(*Signature).tparams = nil
-		}()
-		inst = check.instantiate(pos, typ, tparams, targs, nil)
-	default:
-		// only types and functions can be generic
-		panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ))
 	}
 
-	if verify {
+	return inst, err
-		if check == nil {
+}
-			panic("cannot have nil Checker if verifying constraints")
+
+// instantiate creates an instance and defers verification of constraints to
+// later in the type checking pass. For Named types the resulting instance will
+// be unexpanded.
+func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, posList []token.Pos) (res Type) {
+	assert(check != nil)
+	if trace {
+		check.trace(pos, "-- instantiating %s with %s", typ, typeListString(targs))
+		check.indent++
+		defer func() {
+			check.indent--
+			var under Type
+			if res != nil {
+				// Calling under() here may lead to endless instantiations.
+				// Test case: type T[P any] T[P]
+				// TODO(gri) investigate if that's a bug or to be expected.
+				under = safeUnderlying(res)
 			}
+			check.trace(pos, "=> %s (under = %s)", res, under)
+		}()
+	}
+
+	inst := check.instance(pos, typ, targs)
+
 	assert(len(posList) <= len(targs))
 	check.later(func() {
 		// Collect tparams again because lazily loaded *Named types may not have
@@ -84,51 +96,17 @@ func (check *Checker) Instantiate(pos token.Pos, typ Type, targs []Type, posList
 			}
 		}
 	})
-	}
-
 	return inst
 }
 
-func (check *Checker) instantiate(pos token.Pos, typ Type, tparams []*TypeName, targs []Type, typMap map[string]*Named) (res Type) {
+// instance creates a type or function instance using the given original type
-	// the number of supplied types must match the number of type parameters
+// typ and arguments targs. For Named types the resulting instance will be
-	if len(targs) != len(tparams) {
+// unexpanded.
-		// TODO(gri) provide better error message
+func (check *Checker) instance(pos token.Pos, typ Type, targs []Type) (res Type) {
-		if check != nil {
-			check.errorf(atPos(pos), _Todo, "got %d arguments but %d type parameters", len(targs), len(tparams))
-			return Typ[Invalid]
-		}
-		panic(fmt.Sprintf("%v: got %d arguments but %d type parameters", pos, len(targs), len(tparams)))
-	}
-
-	if check != nil && trace {
-		check.trace(pos, "-- instantiating %s with %s", typ, typeListString(targs))
-		check.indent++
-		defer func() {
-			check.indent--
-			var under Type
-			if res != nil {
-				// Calling under() here may lead to endless instantiations.
-				// Test case: type T[P any] T[P]
-				// TODO(gri) investigate if that's a bug or to be expected.
-				under = safeUnderlying(res)
-			}
-			check.trace(pos, "=> %s (under = %s)", res, under)
-		}()
-	}
-
 	// TODO(gri) What is better here: work with TypeParams, or work with TypeNames?
-
+	switch t := typ.(type) {
-	if len(tparams) == 0 {
+	case *Named:
-		return typ // nothing to do (minor optimization)
+		h := instantiatedHash(t, targs)
-	}
-
-	return check.subst(pos, typ, makeSubstMap(tparams, targs), typMap)
-}
-
-// instantiateLazy avoids actually instantiating the type until needed. typ
-// must be a *Named type.
-func (check *Checker) instantiateLazy(pos token.Pos, orig *Named, targs []Type) Type {
-	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.
@@ -137,16 +115,61 @@ func (check *Checker) instantiateLazy(pos token.Pos, orig *Named, targs []Type)
 			}
 		}
 
-	tname := NewTypeName(pos, orig.obj.pkg, orig.obj.name, nil)
+		tname := NewTypeName(pos, t.obj.pkg, t.obj.name, nil)
-	named := check.newNamed(tname, orig, nil, nil, nil) // methods and tparams are set when named is loaded
+		named := check.newNamed(tname, t, nil, nil, nil) // methods and tparams are set when named is loaded
 		named.targs = targs
 		named.instance = &instance{pos}
-
 		if check != nil {
 			check.typMap[h] = named
 		}
+		res = named
+	case *Signature:
+		tparams := t.TParams()
+		if !check.validateTArgLen(pos, tparams, targs) {
+			return Typ[Invalid]
+		}
+		if tparams.Len() == 0 {
+			return typ // nothing to do (minor optimization)
+		}
+		defer func() {
+			// If we had an unexpected failure somewhere don't panic below when
+			// asserting res.(*Signature). Check for *Signature in case Typ[Invalid]
+			// is returned.
+			if _, ok := res.(*Signature); !ok {
+				return
+			}
+			// If the signature doesn't use its type parameters, subst
+			// will not make a copy. In that case, make a copy now (so
+			// we can set tparams to nil w/o causing side-effects).
+			if t == res {
+				copy := *t
+				res = &copy
+			}
+			// After instantiating a generic signature, it is not generic
+			// anymore; we need to set tparams to nil.
+			res.(*Signature).tparams = nil
+		}()
+		res = check.subst(pos, typ, makeSubstMap(tparams.list(), targs), nil)
+	default:
+		// only types and functions can be generic
+		panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ))
+	}
+	return res
+}
 
-	return named
+// validateTArgLen verifies that the length of targs and tparams matches,
+// reporting an error if not. If validation fails and check is nil,
+// validateTArgLen panics.
+func (check *Checker) validateTArgLen(pos token.Pos, tparams *TParamList, targs []Type) bool {
+	if len(targs) != tparams.Len() {
+		// TODO(gri) provide better error message
+		if check != nil {
+			check.errorf(atPos(pos), _Todo, "got %d arguments but %d type parameters", len(targs), tparams.Len())
+			return false
+		}
+		panic(fmt.Sprintf("%v: got %d arguments but %d type parameters", pos, len(targs), tparams.Len()))
+	}
+	return true
 }
 
 func (check *Checker) verify(pos token.Pos, tparams []*TypeName, targs []Type) (int, error) {

src/go/types/named.go

@@ -262,6 +262,8 @@ func (n *Named) expand(typMap map[string]*Named) *Named {
 		// tparams. This is done implicitly by the call to n.TParams, but making it
 		// explicit is harmless: load is idempotent.
 		n.load()
+		var u Type
+		if n.check.validateTArgLen(n.instance.pos, n.tparams, n.targs) {
 			if typMap == nil {
 				if n.check != nil {
 					typMap = n.check.typMap
@@ -274,10 +276,12 @@ func (n *Named) expand(typMap map[string]*Named) *Named {
 					typMap = map[string]*Named{h: n}
 				}
 			}
-
+			u = n.check.subst(n.instance.pos, n.orig.underlying, makeSubstMap(n.TParams().list(), n.targs), typMap)
-		inst := n.check.instantiate(n.instance.pos, n.orig.underlying, n.TParams().list(), n.targs, typMap)
+		} else {
-		n.underlying = inst
+			u = Typ[Invalid]
-		n.fromRHS = inst
+		}
+		n.underlying = u
+		n.fromRHS = u
 		n.instance = nil
 	}
 	return n

src/go/types/subst.go

@@ -38,13 +38,12 @@ func (m substMap) lookup(tpar *TypeParam) Type {
 	return tpar
 }
 
-// subst returns the type typ with its type parameters tpars replaced by
+// subst returns the type typ with its type parameters tpars replaced by the
-// the corresponding type arguments targs, recursively.
+// corresponding type arguments targs, recursively. subst is pure in the sense
-// subst is functional in the sense that it doesn't modify the incoming
+// that it doesn't modify the incoming type. If a substitution took place, the
-// type. If a substitution took place, the result type is different from
+// result type is different from from the incoming type.
-// from the incoming type.
 //
-// If the given typMap is nil and check is non-nil, check.typMap is used.
+// If the given typMap is non-nil, it is used in lieu of check.typMap.
 func (check *Checker) subst(pos token.Pos, typ Type, smap substMap, typMap map[string]*Named) Type {
 	if smap.empty() {
 		return typ
@@ -157,9 +156,6 @@ func (subst *subster) typ(typ Type) Type {
 		embeddeds, ecopied := subst.typeList(t.embeddeds)
 		if mcopied || ecopied {
 			iface := &Interface{methods: methods, embeddeds: embeddeds, complete: t.complete}
-			if subst.check == nil {
-				panic("internal error: cannot instantiate interfaces yet")
-			}
 			return iface
 		}
 

src/go/types/typexpr.go

@@ -433,7 +433,7 @@ func (check *Checker) instantiatedType(x ast.Expr, targsx []ast.Expr, def *Named
 		posList[i] = arg.Pos()
 	}
 
-	typ := check.Instantiate(x.Pos(), base, targs, posList, true)
+	typ := check.instantiate(x.Pos(), base, targs, posList)
 	def.setUnderlying(typ)
 
 	// make sure we check instantiation works at least once