From 67b83f1382478c890a7593be6ffc1ba69fd96a67 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 12 Dec 2019 16:25:36 -0800 Subject: [PATCH] go/types: represent type parameter bounds always as interfaces When type-checking contracts, construct a parameterized named interface for each of the contract parameters. Use those iterfaces as type bounds for the type parameters. This simplifies and cleans up the implementation. Change-Id: I688e38542fa7fa83dd436439dd9ebe35af2f0458 --- src/go/types/NOTES | 5 ++++ src/go/types/builtins.go | 25 +++++++------------- src/go/types/contracts.go | 35 ++++++++++++++------------- src/go/types/decl.go | 8 +++---- src/go/types/lookup.go | 2 +- src/go/types/subst.go | 8 +------ src/go/types/type.go | 48 ++++++++++---------------------------- src/go/types/typestring.go | 8 ++++--- src/go/types/typexpr.go | 6 ++++- 9 files changed, 60 insertions(+), 85 deletions(-) diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 895ba3c1c1..677c9a3b8b 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -6,6 +6,8 @@ TODO (need an allTypes list, like we have an allMethods list?) OPEN ISSUES +- using a contract and enumerating type arguments currently leads to an error (e.g. func f(type T C(T)) (x T) ... ) +- the above problem is the underlying problem why contracts are not properly "instantiated" when used as type bounds - contracts slip through in places where only types are permitted - conversions against parameterized types are not implemented - parameterized interface methods (of type bounds) need to be customized (subst) for context @@ -16,3 +18,6 @@ DESIGN DECISIONS More generally: Only permit type instantiation T(x) in type context, when the type is a named type. Do not permit in in general in type context: e.g., disallow []T(x) because we consider that a conversion, in general. Same for ([]T)(x). + +- 12/12/2019: represent type bounds always as (possibly unnamed) interfaces + (contracts are for user only) diff --git a/src/go/types/builtins.go b/src/go/types/builtins.go index c91bf04b21..a68da1205f 100644 --- a/src/go/types/builtins.go +++ b/src/go/types/builtins.go @@ -648,11 +648,11 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b } // applyTypeFunc applies f to x. If x is a type parameter, -// the result is an anonymous type parameter constrained by -// an unnamed contract. The type constraints of that contract -// are computed by applying f to each of the contract type -// constraints of x. If any of these applications of f return -// nil, applyTypeFunc returns nil. +// the result is a type parameter constrained by an new +// interface bound. The type bounds for that interface +// are computed by applying f to each of the type bounds +// of x. If any of these applications of f return nil, +// applyTypeFunc returns nil. // If x is not a type parameter, the result is f(x). func (check *Checker) applyTypeFunc(f func(Type) Type, x Type) Type { if tp, _ := x.Underlying().(*TypeParam); tp != nil { @@ -675,18 +675,11 @@ func (check *Checker) applyTypeFunc(f func(Type) Type, x Type) Type { // define type and initialize a variable? // construct a suitable new type parameter - tpar := NewTypeName(token.NoPos, nil /* = Universe pkg */, "", nil) - resTyp := check.NewTypeParam(tpar, 0, nil) // assigns type to tpar as a side-effect + tpar := NewTypeName(token.NoPos, nil /* = Universe pkg */, "", nil) + ptyp := check.NewTypeParam(tpar, 0, nil) // assigns type to tpar as a side-effect + ptyp.bound = &Interface{allMethods: markComplete, types: resTypes} - // construct a corresponding interface and contract - iface := &Interface{allMethods: markComplete, types: resTypes} - contr := &Contract{ - TParams: []*TypeName{tpar}, - IFaces: map[*TypeName]*Interface{tpar: iface}, - } - resTyp.bound = contr - - return resTyp + return ptyp } return f(x) diff --git a/src/go/types/contracts.go b/src/go/types/contracts.go index a7966891f6..ab05a14182 100644 --- a/src/go/types/contracts.go +++ b/src/go/types/contracts.go @@ -14,7 +14,7 @@ import ( // TODO(gri) Handling a contract like a type is problematic because it // won't exclude a contract where we only permit a type. Investigate. -func (check *Checker) contractType(contr *Contract, e *ast.ContractType) { +func (check *Checker) contractType(contr *Contract, name string, e *ast.ContractType) { check.openScope(e, "contract") defer check.closeScope() @@ -27,15 +27,22 @@ func (check *Checker) contractType(contr *Contract, e *ast.ContractType) { tparams[index] = tpar } - // each type parameter's constraints are represented by a (lazily allocated) interface - ifaces := make(map[*TypeName]*Interface) + // TODO(gri) review this - we probably don't need lazy allocation anymore + // Each type parameter's constraints are represented by a (lazily allocated) named interface. + // Given a contract C(P1, P2, ... Pn) { ... } we construct named types C1(P1, P2, ... Pn), + // C2(P1, P2, ... Pn), ... Cn(P1, P2, ... Pn) with the respective underlying interfaces + // representing the type constraints for each of the type parameters (C1 for P1, C2 for P2, etc.). + bounds := make(map[*TypeName]*Named) ifaceFor := func(tpar *TypeName) *Interface { - iface := ifaces[tpar] - if iface == nil { - iface = new(Interface) - ifaces[tpar] = iface + named := bounds[tpar] + if named == nil { + index := tpar.typ.(*TypeParam).index + tname := NewTypeName(e.Pos(), check.pkg, name+string(subscript(uint64(index))), nil) + tname.tparams = tparams + named = NewNamed(tname, new(Interface), nil) + bounds[tpar] = named } - return iface + return named.underlying.(*Interface) } // collect constraints @@ -131,17 +138,13 @@ func (check *Checker) contractType(contr *Contract, e *ast.ContractType) { } } - // cleanup/complete interfaces - for tpar, iface := range ifaces { - if iface == nil { - ifaces[tpar] = &emptyInterface - } else { - check.completeInterface(e.Pos(), iface) - } + // complete interfaces + for _, bound := range bounds { + check.completeInterface(e.Pos(), bound.underlying.(*Interface)) } contr.TParams = tparams - contr.IFaces = ifaces + contr.Bounds = bounds } func (check *Checker) collectTypeConstraints(pos token.Pos, list []Type, types []ast.Expr) []Type { diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 54854f82d5..2763d9dd52 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -614,7 +614,7 @@ func (check *Checker) collectTypeParams(list *ast.FieldList) (tparams []*TypeNam index = 0 for _, f := range list.List { - var bound Type + var bound Type = &emptyInterface var isContract bool if f.Type != nil { typ := check.typ(f.Type) @@ -641,10 +641,8 @@ func (check *Checker) collectTypeParams(list *ast.FieldList) (tparams []*TypeNam for i, name := range f.Names { tname := tparams[index] assert(name.Name == tname.name) // catch index errors - // TODO(gri) enable the then branch to force interfaces for all - // bounds and unpack the interface in the else branch - if false && isContract { - tname.typ.(*TypeParam).bound = bound.Underlying().(*Contract).ifaceAt(i) + if isContract { + tname.typ.(*TypeParam).bound = bound.Underlying().(*Contract).boundsAt(i) } else { tname.typ.(*TypeParam).bound = bound } diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index 78b0f52f9c..bb8e5edad0 100644 --- a/src/go/types/lookup.go +++ b/src/go/types/lookup.go @@ -363,7 +363,7 @@ func deref(typ Type) (Type, bool) { func derefUnpack(typ Type) (Type, bool) { typ, ptr := deref(typ) if tpar, _ := typ.(*TypeParam); tpar != nil { - typ = tpar.Interface() + typ = tpar.bound } return typ, ptr } diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 40da37154a..12fbc42cea 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -78,13 +78,7 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist // TODO(gri) document/explain why the substitution below is correct //check.dump(">>> %s: iface before: %s", pos, iface) - bound := check.subst(pos, tpar.bound, tparams, targs).Underlying() - switch b := bound.(type) { - case *Interface: - iface = b - case *Contract: - iface = b.ifaceAt(i) - } + iface = check.subst(pos, iface, tparams, targs).(*Interface) //check.dump(">>> %s: iface after : %s", pos, iface) // targ must implement iface (methods) diff --git a/src/go/types/type.go b/src/go/types/type.go index e8aa697b9e..2105b52b09 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -513,34 +513,24 @@ func (t *Named) AddMethod(m *Func) { } // A Contract represents a contract. -// TODO(gri) Do we need the ability to represent unnamed type parameter literals? -// For instance, when creating (result) type parameters out of whole cloth -// say for the result type of real/imag(x) where x is of a parameterized type. type Contract struct { - TParams []*TypeName - IFaces map[*TypeName]*Interface + TParams []*TypeName // type parameters in declaration order + Bounds map[*TypeName]*Named // underlying type is always *Interface } -// ifaceAt returns the interface matching for the respective -// contract type parameter with the given index. If c is nil -// the result is the empty interface. -func (c *Contract) ifaceAt(index int) *Interface { - var iface *Interface - if c != nil { - iface = c.IFaces[c.TParams[index]] - } - if iface == nil { - iface = &emptyInterface - } - return iface +// boundsAt returns the (named) interface for the respective +// contract type parameter with the given index. +// TODO(gri) we probably don't need this one +func (c *Contract) boundsAt(index int) *Named { + return c.Bounds[c.TParams[index]] } // A TypeParam represents a type parameter type. type TypeParam struct { - id uint64 // unique id (TODO should this be with the object? all objects?) - obj *TypeName - index int // parameter index - bound Type // either an *Interface or a *Contract // TODO(gri) make this *Interface or nil only? + id uint64 // unique id (TODO should this be with the object? all objects?) + obj *TypeName // corresponding type name + index int // parameter index + bound Type // *Named or *Interface; underlying type is always *Interface } // NewTypeParam returns a new TypeParam. @@ -553,22 +543,8 @@ func (check *Checker) NewTypeParam(obj *TypeName, index int, bound Type) *TypePa return typ } -// Interface returns the type parameter's interface as -// specified via its contract. If there is no contract, -// the result is the empty interface. -// TODO(gri) should this be Underlying instead? func (t *TypeParam) Interface() *Interface { - if t.bound == nil { - return &emptyInterface - } - switch b := t.bound.Underlying().(type) { - case *Interface: - return b - case *Contract: - return b.ifaceAt(t.index) - default: - panic("type parameter bound must be an interface or contract") - } + return t.bound.Underlying().(*Interface) } // Implementations for Type methods. diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go index 86651e3d3e..1b60c3e3f7 100644 --- a/src/go/types/typestring.go +++ b/src/go/types/typestring.go @@ -265,7 +265,7 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) { // if i > 0 { // buf.WriteString(", ") // } - // writeTypeName(buf, tpar, qf) + // writeType(buf, tpar.typ, qf, visited) // } // buf.WriteByte(')') // } @@ -280,13 +280,15 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) { } buf.WriteString("){") i := 0 - for tpar, iface := range t.IFaces { + for tpar, bound := range t.Bounds { if i > 0 { buf.WriteString("; ") } writeType(buf, tpar.typ, qf, visited) buf.WriteByte(' ') - writeType(buf, iface, qf, visited) + writeType(buf, bound, qf, visited) + buf.WriteString(" = ") + writeType(buf, bound.underlying, qf, visited) i++ } buf.WriteByte('}') diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 2085abdbb9..3dc70e92e8 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -376,7 +376,11 @@ func (check *Checker) typInternal(e ast.Expr, def *Named) (T Type) { case *ast.ContractType: typ := new(Contract) def.setUnderlying(typ) - check.contractType(typ, e) + name := "" + if def != nil { + name = def.obj.name + } + check.contractType(typ, name, e) return typ default: