From eda3de0f79f3008aed21c04df546efa2ace65683 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 13 Aug 2021 13:15:15 -0400 Subject: [PATCH] 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 Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- .../compile/internal/types2/instantiate.go | 69 ++++++++++--------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go index ef92574a5c..ab51d062c5 100644 --- a/src/cmd/compile/internal/types2/instantiate.go +++ b/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) { - 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 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 false + return errorf("%s has no constraints", targ) } - check.softErrorf(pos, "%s does not satisfy comparable", targ) - return false + return errorf("%s does not satisfy comparable", targ) } // 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 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! @@ -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, - "%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(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 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(pos, "%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(pos, "%s does not satisfy %s", targ, tpar.bound) - return false + return errorf("%s does not satisfy %s", targ, tpar.bound) } - return true + return nil }