From 0de29c0b762334b7132c8bee1520ba8d0d53b1b0 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 9 Dec 2019 16:15:15 -0800 Subject: [PATCH] go/types: more cleanups around type instantiation - type instantiations checks parameter count - better positions for error messages - removed some (now) dead code Change-Id: Icf70642bbfd4e45a7762b002ea94704dc3d56475 --- src/go/types/NOTES | 1 - src/go/types/call.go | 9 +++- src/go/types/expr.go | 2 +- src/go/types/subst.go | 17 +++++++- src/go/types/testdata/contracts.go2 | 10 ++--- src/go/types/testdata/tmp.go2 | 2 +- src/go/types/typexpr.go | 68 +++-------------------------- 7 files changed, 35 insertions(+), 74 deletions(-) diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 39620f4031..24de144410 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -1,6 +1,5 @@ TODO - satisfyBounds is not working correctly -- no bounds checks in instantiate call (see open issues below) - allow recursive type parameterization without need to repeat type parameters - if type parameters are repeated in recursive instantiation, they must be the same order - implement contract embedding diff --git a/src/go/types/call.go b/src/go/types/call.go index 2b6fb5fd18..2f64d364b3 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -85,6 +85,7 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { // if the first argument is a type, assume we have explicit type arguments // we must have the correct number of type parameters + // TODO(gri) do this in the instantiate call? if n != len(sig.tparams) { check.errorf(args[n-1].pos(), "got %d type arguments but want %d", n, len(sig.tparams)) x.mode = invalid @@ -94,6 +95,7 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { // collect types targs := make([]Type, n) + poslist := make([]token.Pos, n) for i, a := range args { if a.mode != typexpr { // error was reported earlier @@ -102,10 +104,11 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { return expression } targs[i] = a.typ + poslist[i] = a.pos() } // instantiate function signature - x.typ = check.instantiate(x.pos(), sig, targs) + x.typ = check.instantiate(x.pos(), sig, targs, poslist) x.mode = value x.expr = e return expression @@ -309,11 +312,13 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, args []*oper // infer type arguments and instantiate signature if necessary if len(sig.tparams) > 0 { + // TODO(gri) provide position information for targs so we can feed + // it to the instantiate call for better error reporting targs := check.infer(call.Rparen, sig.tparams, params, args) if targs == nil { return } - rsig = check.instantiate(call.Pos(), sig, targs).(*Signature) + rsig = check.instantiate(call.Pos(), sig, targs, nil).(*Signature) params = check.subst(call.Pos(), params, sig.tparams, targs).(*Tuple) // TODO(gri) Optimization: We don't need to check arguments // from which we inferred parameter types. diff --git a/src/go/types/expr.go b/src/go/types/expr.go index 83a7142cee..3361f88438 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -1515,7 +1515,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType, *ast.ContractType: x.mode = typexpr - x.typ = check.typ(e) // TODO(gri) should this be check.instantiatedType? + x.typ = check.typ(e) // Note: rawExpr (caller of exprInternal) will call check.recordTypeAndValue // even though check.typ has already called it. This is fine as both // times the same expression and type are recorded. It is also not a diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 3eac9ba021..aa7ec979d6 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -14,7 +14,7 @@ import ( "strings" ) -func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type) (res Type) { +func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist []token.Pos) (res Type) { if check.conf.Trace { check.trace(pos, "-- instantiating %s with %s", typ, typeListString(targs)) check.indent++ @@ -28,6 +28,8 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type) (res Ty }() } + assert(poslist == nil || len(poslist) == len(targs)) + // TODO(gri) What is better here: work with TypeParams, or work with TypeNames? var tparams []*TypeName switch typ := typ.(type) { @@ -41,6 +43,13 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type) (res Ty } + // the number of supplied types must match the number of type parameters + if len(targs) != len(tparams) { + // TODO(gri) provide better error message + check.errorf(pos, "got %d arguments but %d type parameters", len(targs), len(tparams)) + return Typ[Invalid] + } + // check bounds for i, tname := range tparams { tpar := tname.typ.(*TypeParam) @@ -48,6 +57,12 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type) (res Ty continue // no bound } + // best position for error reporting + pos := pos + if i < len(poslist) { + pos = poslist[i] + } + // determine type parameter bound var iface *Interface switch bound := tpar.bound.Underlying().(type) { diff --git a/src/go/types/testdata/contracts.go2 b/src/go/types/testdata/contracts.go2 index 81a55650aa..08348a7d6b 100644 --- a/src/go/types/testdata/contracts.go2 +++ b/src/go/types/testdata/contracts.go2 @@ -83,7 +83,7 @@ type A struct{} func (A) a() {} -var _ T1(int /* ERROR not satisfied */ , int) +var _ T1(int /* ERROR does not satisfy */ , int) var _ T1(A, int) contract Stringer(T) { @@ -96,7 +96,7 @@ type List(type T Stringer) struct{ } var _ List(MyData) -var _ List(int /* ERROR not satisfied */ ) +var _ List(int /* ERROR does not satisfy */ ) type MyData string @@ -110,10 +110,10 @@ contract C3(T) { type T2 (type _ C3) struct{} -var _ T2(int8 /* ERROR not satisfied */ ) +var _ T2(int8 /* ERROR does not satisfy */ ) var _ T2(int16) var _ T2(int32) -var _ T2(int64 /* ERROR not satisfied */ ) +var _ T2(int64 /* ERROR does not satisfy */ ) var _ T2(int) var _ T2(struct{x int}) @@ -192,7 +192,7 @@ type B1 interface{type int} func f1(type P B1)(x P) {} func _() { - f1 /* ERROR string not found */ (string)("foo") + f1(string /* ERROR string not found */ )("foo") f1 /* ERROR string not found */ ("foo") f1 /* ERROR float64 not found */ (1.2) f1(42) diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 6062d388f4..3325ef93a4 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -10,7 +10,7 @@ contract C(T) { type Cm(type T C) T -func (a Cm(T /* ERROR not satisfied */ )) m() T +func (a Cm(T /* ERROR does not satisfy */ )) m() T // TODO(gri) make this work /* diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 0661fe8ca3..4909d77041 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -278,41 +278,12 @@ func (check *Checker) typInternal(e ast.Expr, def *Named) (T Type) { return Typ[Invalid] } - // the number of supplied types must match the number of type parameters - tname := typ.(*Named).obj // generic types are defined (= Named) types - if len(targs) != len(tname.tparams) { - // TODO(gri) provide better error message - check.errorf(e.Pos(), "got %d arguments but %d type parameters", len(e.Args), len(tname.tparams)) - return Typ[Invalid] - } - - // substitute type bound parameters with arguments - // and check if each argument satisfies its bound - for i, tpar := range tname.tparams { - pos := e.Args[i].Pos() - bound := tpar.typ.(*TypeParam).bound // interface or contract or nil - if bound == nil { - continue // nothing to do - } - switch b := bound.Underlying().(type) { - case *Interface: - iface := check.subst(token.NoPos, b, tname.tparams, targs).(*Interface) - if !check.satisfyBound(pos, tpar, targs[i], iface) { - return Typ[Invalid] - } - case *Contract: - // check.dump("### iface = %s, tparams = %s, targs = %s", b.ifaceAt(i), tname.tparams, targs) - iface := check.subst(token.NoPos, b.ifaceAt(i), tname.tparams, targs).(*Interface) - if !check.satisfyBound(pos, tpar, targs[i], iface) { - return Typ[Invalid] - } - default: - unreachable() - } - } - // instantiate parameterized type - typ = check.instantiate(e.Pos(), typ, targs) + poslist := make([]token.Pos, len(e.Args)) + for i, arg := range e.Args { + poslist[i] = arg.Pos() + } + typ = check.instantiate(e.Pos(), typ, targs, poslist) def.setUnderlying(typ) return typ @@ -485,35 +456,6 @@ func (check *Checker) typeList(list []ast.Expr) []Type { return res } -func (check *Checker) satisfyBound(pos token.Pos, tname *TypeName, arg Type, bound *Interface) bool { - // check.dump("### satisfyBound: tname = %s, arg = %s, bound = %s", tname, arg, bound) - // use interface type of type parameter, if any - // targ must implement iface - if m, _ := check.missingMethod(arg, bound, true); m != nil { - check.errorf(pos, "constraint for %s is not satisfied", tname) - // check.dump("missing %s (%s, %s)", m, arg, bound) - return false - } - // arg's underlying type must also be one of the bound interface types listed, if any - if len(bound.types) > 0 { - utyp := arg.Underlying() - // TODO(gri) Cannot handle a type argument that is itself parameterized for now - switch utyp.(type) { - case *Interface, *Contract: - panic("unimplemented") - } - for _, t := range bound.types { - // if we find one matching type, we're ok - if Identical(utyp, t) { - return true - } - } - check.errorf(pos, "constraint for %s is not satisfied (not an enumerated type)", tname) - return false - } - return true -} - func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicOk bool) (params []*Var, variadic bool) { if list == nil { return