go/types: change Checker.verify to return an error

This is a port of CL 342151 to go/types, adjusted for errors and
positions. Checker.sprintf was refactored to facilitate formatting
error messages with a nil Checker.

Change-Id: Ib2e5c942e55edaff7b5e77cf68a72bad70fea0b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/342670
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/go/types/errors.go b/src/go/types/errors.go
index 7468626b98618c6a1a41b849f7ae92ead5575727..933de93d8585b05d32c8e217e183876e6d5f0dfa 100644 (file)
--- a/src/go/types/errors.go
+++ b/src/go/types/errors.go
@@ -63,6 +63,10 @@ func (check *Checker) markImports(pkg *Package) {
 }
 
 func (check *Checker) sprintf(format string, args ...interface{}) string {
+       return sprintf(check.fset, check.qualifier, format, args...)
+}
+
+func sprintf(fset *token.FileSet, qf Qualifier, format string, args ...interface{}) string {
        for i, arg := range args {
                switch a := arg.(type) {
                case nil:
@@ -70,15 +74,17 @@ func (check *Checker) sprintf(format string, args ...interface{}) string {
                case operand:
                        panic("got operand instead of *operand")
                case *operand:
-                       arg = operandString(a, check.qualifier)
+                       arg = operandString(a, qf)
                case token.Pos:
-                       arg = check.fset.Position(a).String()
+                       if fset != nil {
+                               arg = fset.Position(a).String()
+                       }
                case ast.Expr:
                        arg = ExprString(a)
                case Object:
-                       arg = ObjectString(a, check.qualifier)
+                       arg = ObjectString(a, qf)
                case Type:
-                       arg = TypeString(a, check.qualifier)
+                       arg = TypeString(a, qf)
                }
                args[i] = arg
        }

diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go
index eeb9b03050f93f06f2403091dedf64a35a094786..86e5e202c48b5f3fa5ef5e3dbbab8b2ee88d8408 100644 (file)
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -8,6 +8,7 @@
 package types
 
 import (
+       "errors"
        "fmt"
        "go/token"
 )
@@ -73,7 +74,14 @@ func (check *Checker) Instantiate(pos token.Pos, typ Type, targs []Type, posList
                        // 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(atPos(pos), _Todo, err.Error())
+                               }
                        }
                })
        }
@@ -141,30 +149,36 @@ func (check *Checker) instantiateLazy(pos token.Pos, orig *Named, targs []Type)
        return named
 }
 
-func (check *Checker) verify(pos token.Pos, tparams []*TypeName, targs []Type, posList []token.Pos) {
+func (check *Checker) verify(pos token.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) {
-                       break
+               if err := check.satisfies(pos, targs[i], tname.typ.(*TypeParam), smap); err != nil {
+                       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 token.Pos, targ Type, tpar *TypeParam, smap substMap) bool {
+func (check *Checker) satisfies(pos token.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(nil, qf, format, args...))
        }
 
        // The type parameter bound is parameterized with the same type parameters
@@ -177,11 +191,9 @@ func (check *Checker) satisfies(pos token.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(atPos(pos), _Todo, "%s has no constraints", targ)
-                       return false
+                       return errorf("%s has no constraints", targ)
                }
-               check.softErrorf(atPos(pos), _Todo, "%s does not satisfy comparable", targ)
-               return false
+               return errorf("%s does not satisfy comparable", targ)
        }
 
        // targ must implement iface (methods)
@@ -191,8 +203,7 @@ func (check *Checker) satisfies(pos token.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(atPos(pos), 0, "%s has no methods", targ)
-                       return false
+                       return errorf("%s has no methods", targ)
                }
                if m, wrong := check.missingMethod(targ, iface, true); m != nil {
                        // TODO(gri) needs to print updated name to avoid major confusion in error message!
@@ -203,20 +214,17 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
                                // TODO(gri) This can still report uninstantiated types which makes the error message
                                //           more difficult to read then necessary.
                                // TODO(rFindley) should this use parentheses rather than ':' for qualification?
-                               check.softErrorf(atPos(pos), _Todo,
-                                       "%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
+                               return errorf("%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
                                        targ, tpar.bound, wrong, m,
                                )
-                       } else {
-                               check.softErrorf(atPos(pos), 0, "%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
@@ -224,23 +232,20 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
        if targ := asTypeParam(targ); targ != nil {
                targBound := targ.iface()
                if !targBound.typeSet().hasTerms() {
-                       check.softErrorf(atPos(pos), _Todo, "%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
-                       return false
+                       return errorf("%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
                }
                if !targBound.typeSet().subsetOf(iface.typeSet()) {
                        // TODO(gri) need better error message
-                       check.softErrorf(atPos(pos), _Todo, "%s does not satisfy %s", targ, tpar.bound)
-                       return false
+                       return errorf("%s does not satisfy %s", targ, tpar.bound)
                }
-               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(atPos(pos), _Todo, "%s does not satisfy %s", targ, tpar.bound)
-               return false
+               return errorf("%s does not satisfy %s", targ, tpar.bound)
        }
 
-       return true
+       return nil
 }