From 0f252511272d340c3fa9d25acfcc9ff9d809cd7d Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Mon, 16 Aug 2021 16:15:09 -0400 Subject: [PATCH] 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 Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/types/errors.go | 14 +++++--- src/go/types/instantiate.go | 69 ++++++++++++++++++++----------------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/go/types/errors.go b/src/go/types/errors.go index 7468626b986..933de93d858 100644 --- 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 eeb9b03050f..86e5e202c48 100644 --- 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 }