cmd/compile/internal/types2: change Checker.verify to return an error

In preparation for upcoming API changes, change the internal API for
verification of type arguments to return an error and argument index,
and use this to lift up error reporting into Instantiate.

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

@@ -9,6 +9,7 @@ package types2
 
 import (
 	"cmd/compile/internal/syntax"
+	"errors"
 	"fmt"
 )
 
@@ -74,7 +75,14 @@ func (check *Checker) Instantiate(pos syntax.Pos, typ Type, targs []Type, posLis
 			// Avoid duplicate errors; instantiate will have complained if tparams
 			// and targs do not have the same length.
 			if len(tparams) == len(targs) {
-				check.verify(pos, tparams, targs, posList)
+				if i, err := check.verify(pos, tparams, targs); err != nil {
+					// best position for error reporting
+					pos := pos
+					if i < len(posList) {
+						pos = posList[i]
+					}
+					check.softErrorf(pos, err.Error())
+				}
 			}
 		})
 	}
@@ -139,30 +147,36 @@ func (check *Checker) instantiateLazy(pos syntax.Pos, orig *Named, targs []Type)
 	return named
 }
 
-func (check *Checker) verify(pos syntax.Pos, tparams []*TypeName, targs []Type, posList []syntax.Pos) {
+func (check *Checker) verify(pos syntax.Pos, tparams []*TypeName, targs []Type) (int, error) {
 	smap := makeSubstMap(tparams, targs)
 	for i, tname := range tparams {
-		// best position for error reporting
-		pos := pos
-		if i < len(posList) {
-			pos = posList[i]
-		}
-
 		// stop checking bounds after the first failure
-		if !check.satisfies(pos, targs[i], tname.typ.(*TypeParam), smap) {
+		if err := check.satisfies(pos, targs[i], tname.typ.(*TypeParam), smap); err != nil {
-			break
+			return i, err
 		}
 	}
+	return -1, nil
 }
 
 // satisfies reports whether the type argument targ satisfies the constraint of type parameter
 // parameter tpar (after any of its type parameters have been substituted through smap).
 // A suitable error is reported if the result is false.
 // TODO(gri) This should be a method of interfaces or type sets.
-func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap substMap) bool {
+func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap substMap) error {
 	iface := tpar.iface()
 	if iface.Empty() {
-		return true // no type bound
+		return nil // no type bound
+	}
+
+	// TODO(rfindley): it would be great if users could pass in a qualifier here,
+	// rather than falling back to verbose qualification. Maybe this can be part
+	// of a the shared environment.
+	var qf Qualifier
+	if check != nil {
+		qf = check.qualifier
+	}
+	errorf := func(format string, args ...interface{}) error {
+		return errors.New(sprintf(qf, format, args...))
 	}
 
 	// The type parameter bound is parameterized with the same type parameters
@@ -175,11 +189,9 @@ func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap
 	// TODO(gri) the error messages needs to be better, here
 	if iface.IsComparable() && !Comparable(targ) {
 		if tpar := asTypeParam(targ); tpar != nil && tpar.iface().typeSet().IsAll() {
-			check.softErrorf(pos, "%s has no constraints", targ)
+			return errorf("%s has no constraints", targ)
-			return false
 		}
-		check.softErrorf(pos, "%s does not satisfy comparable", targ)
+		return errorf("%s does not satisfy comparable", targ)
-		return false
 	}
 
 	// targ must implement iface (methods)
@@ -189,8 +201,7 @@ func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap
 		// method set is empty.
 		// TODO(gri) is this what we want? (spec question)
 		if base, isPtr := deref(targ); isPtr && asTypeParam(base) != nil {
-			check.errorf(pos, "%s has no methods", targ)
+			return errorf("%s has no methods", targ)
-			return false
 		}
 		if m, wrong := check.missingMethod(targ, iface, true); m != nil {
 			// TODO(gri) needs to print updated name to avoid major confusion in error message!
@@ -200,20 +211,17 @@ func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap
 			if wrong != nil {
 				// TODO(gri) This can still report uninstantiated types which makes the error message
 				//           more difficult to read then necessary.
-				check.softErrorf(pos,
+				return errorf("%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
-					"%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
 					targ, tpar.bound, wrong, m,
 				)
-			} else {
-				check.softErrorf(pos, "%s does not satisfy %s (missing method %s)", targ, tpar.bound, m.name)
 			}
-			return false
+			return errorf("%s does not satisfy %s (missing method %s)", targ, tpar.bound, m.name)
 		}
 	}
 
 	// targ's underlying type must also be one of the interface types listed, if any
 	if !iface.typeSet().hasTerms() {
-		return true // nothing to do
+		return nil // nothing to do
 	}
 
 	// If targ is itself a type parameter, each of its possible types, but at least one, must be in the
@@ -221,23 +229,20 @@ func (check *Checker) satisfies(pos syntax.Pos, targ Type, tpar *TypeParam, smap
 	if targ := asTypeParam(targ); targ != nil {
 		targBound := targ.iface()
 		if !targBound.typeSet().hasTerms() {
-			check.softErrorf(pos, "%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
+			return errorf("%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
-			return false
 		}
 		if !targBound.typeSet().subsetOf(iface.typeSet()) {
 			// TODO(gri) need better error message
-			check.softErrorf(pos, "%s does not satisfy %s", targ, tpar.bound)
+			return errorf("%s does not satisfy %s", targ, tpar.bound)
-			return false
 		}
-		return true
+		return nil
 	}
 
 	// Otherwise, targ's type or underlying type must also be one of the interface types listed, if any.
 	if !iface.typeSet().includes(targ) {
 		// TODO(gri) better error message
-		check.softErrorf(pos, "%s does not satisfy %s", targ, tpar.bound)
+		return errorf("%s does not satisfy %s", targ, tpar.bound)
-		return false
 	}
 
-	return true
+	return nil
 }