From 1a93d0a2ca31156ce2ae36402cd99de4980b8441 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 19 Dec 2019 21:41:07 -0800 Subject: [PATCH] go/types: treat contract decl as declaration, not type This change completes the prior change by adjusting the go/types implementation accordingly. As a consequence, contracts may now not occur in type context anymore and errors are reported if they do. With this change, all tests pass again. Change-Id: Id984b3d24b7cb6ff2fceb74fd3c33fd0af91fdce --- src/go/types/NOTES | 5 +++- src/go/types/contracts.go | 20 +++++++------ src/go/types/decl.go | 44 ++++++++++++++++++++--------- src/go/types/expr.go | 2 +- src/go/types/exprstring.go | 6 ---- src/go/types/object.go | 44 +++++++++++++++++++++++++++++ src/go/types/resolver.go | 17 +++++++---- src/go/types/subst.go | 25 +--------------- src/go/types/testdata/contracts.go2 | 17 +++++------ src/go/types/type.go | 15 ---------- src/go/types/typestring.go | 23 --------------- src/go/types/typexpr.go | 15 ++++------ 12 files changed, 117 insertions(+), 116 deletions(-) diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 31015c4390..795d992b2b 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -1,4 +1,6 @@ TODO +- use Underlying() to return a type parameter's bound? investigate! +- better error message when declaring a contract local to a function (parser gets out of sync) - if type parameters are repeated in recursive instantiation, they must be the same order (not yet checked) - implement contract embedding - interface embedding doesn't take care of literal type constraints yet @@ -10,7 +12,6 @@ TODO OPEN ISSUES - using a contract and enumerating type arguments currently leads to an error (e.g. func f(type T C(T)) (x T) ... ) -- contracts slip through in places where only types are permitted DESIGN/IMPLEMENTATION - 12/4/2019: do not allow parenthesized generic uninstantiated types (unless instantiated implicitly) @@ -29,3 +30,5 @@ DESIGN/IMPLEMENTATION is not valid, no matter how T1 and T2 are instantiated (but if T1 and T2 were type aliases with both of them having type int, the return x would be valid). In fact, the type parameters act more like named types. With their underlying type being the interface by which they are bound. + +- 12/19/2019: rewrote contract handling: they are now treated as Objects not Types throughout diff --git a/src/go/types/contracts.go b/src/go/types/contracts.go index d1a2284287..7cbbf1d0da 100644 --- a/src/go/types/contracts.go +++ b/src/go/types/contracts.go @@ -11,10 +11,18 @@ import ( "go/token" ) -// TODO(gri) Handling a contract like a type is problematic because it -// won't exclude a contract where we only permit a type. Investigate. +type contractType struct{} + +func (contractType) String() string { return "" } +func (contractType) Underlying() Type { panic("unreachable") } + +func (check *Checker) contractDecl(contr *Contract, e *ast.ContractSpec) { + assert(contr.typ == nil) + + // contracts don't have types, but we need to set a type to + // detect recursive declrations and satisfy various assertions + contr.typ = new(contractType) -func (check *Checker) contractType(contr *Contract, name string, e *ast.ContractType) { check.openScope(e, "contract") defer check.closeScope() @@ -37,7 +45,7 @@ func (check *Checker) contractType(contr *Contract, name string, e *ast.Contract named := bounds[tpar] if named == nil { index := tpar.typ.(*TypeParam).index - tname := NewTypeName(e.Pos(), check.pkg, name+string(subscript(uint64(index))), nil) + tname := NewTypeName(e.Pos(), check.pkg, contr.name+string(subscript(uint64(index))), nil) tname.tparams = tparams named = NewNamed(tname, new(Interface), nil) bounds[tpar] = named @@ -215,10 +223,6 @@ func (check *Checker) typeConstraint(typ Type, why *string) bool { case *Named: *why = check.sprintf("%s is not a type literal", t) return false - case *Contract: - // TODO(gri) we shouldn't reach here - *why = check.sprintf("%s is not a type", t) - return false case *TypeParam: // TODO(gri) should this be ok? need a good use case // ok for now diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 8b5fab30ee..07377c4e4e 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -197,6 +197,8 @@ func (check *Checker) objDecl(obj Object, def *Named) { case *Func: // functions may be recursive - no need to track dependencies check.funcDecl(obj, d) + case *Contract: + check.contractDecl(obj, d.cdecl) default: unreachable() } @@ -616,24 +618,38 @@ func (check *Checker) collectTypeParams(list *ast.FieldList) (tparams []*TypeNam index += len(f.Names) } + // TODO(gri) clean up this loop - can be written much more cleanly index = 0 for _, f := range list.List { + var contr *Contract var bound Type = &emptyInterface if f.Type != nil { - typ := check.typ(f.Type) - if typ != Typ[Invalid] { - switch b := typ.Underlying().(type) { - case *Interface: - bound = typ - case *Contract: - if len(f.Names) != len(b.TParams) { + // If f.Type denotes a contract, handle everything here so we don't + // need to set up a special contract mode for operands just to carry + // its information through in form of some contract Type. + // TODO(gri) should we allow parenthesized contracts? what if there are contract arguments? + if ident, ok := f.Type.(*ast.Ident); ok { + obj := check.lookup(ident.Name) + if contr, _ = obj.(*Contract); contr != nil { + if len(f.Names) != len(contr.TParams) { // TODO(gri) improve error message - check.errorf(f.Type.Pos(), "%d type parameters but contract expects %d", len(f.Names), len(b.TParams)) + check.errorf(f.Type.Pos(), "%d type parameters but contract expects %d", len(f.Names), len(contr.TParams)) + // TODO(gri) in this case we don't set any bound - make this uniform break // cannot use this contract } - bound = typ - default: - check.errorf(f.Type.Pos(), "%s is not an interface or contract", typ) + } + } + + if contr == nil { + typ := check.typ(f.Type) + if typ != Typ[Invalid] { + if b, _ := typ.Underlying().(*Interface); b != nil { + bound = typ + } else { + // TODO(gri) same here: in this case we don't set any bound - make this uniform + check.errorf(f.Type.Pos(), "%s is not an interface or contract", typ) + break + } } } } @@ -645,11 +661,11 @@ func (check *Checker) collectTypeParams(list *ast.FieldList) (tparams []*TypeNam // If we have a contract, use its matching type parameter bound // and instantiate it with the actual type parameters (== arguments) // present. - bound := bound - if b, _ := bound.Underlying().(*Contract); b != nil { + bound := bound // TODO(gri) this is confusing - rewrite + if contr != nil { // TODO(gri) eliminate this nil test by ensuring that all // contract parameters have an associated bound - if bat := b.boundsAt(i); bat != nil { + if bat := contr.boundsAt(i); bat != nil { if targs == nil { targs = make([]Type, len(f.Names)) for i, tparam := range tparams[index : index+len(f.Names)] { diff --git a/src/go/types/expr.go b/src/go/types/expr.go index ccb1a3223b..bd037e2abf 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -1546,7 +1546,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { goto Error case *ast.ArrayType, *ast.StructType, *ast.FuncType, - *ast.InterfaceType, *ast.MapType, *ast.ChanType, *ast.ContractType: + *ast.InterfaceType, *ast.MapType, *ast.ChanType: x.mode = typexpr x.typ = check.typ(e) // Note: rawExpr (caller of exprInternal) will call check.recordTypeAndValue diff --git a/src/go/types/exprstring.go b/src/go/types/exprstring.go index c6c44b6da6..87cd9c57b4 100644 --- a/src/go/types/exprstring.go +++ b/src/go/types/exprstring.go @@ -166,12 +166,6 @@ func WriteExpr(buf *bytes.Buffer, x ast.Expr) { } buf.WriteString(s) WriteExpr(buf, x.Value) - - case *ast.ContractType: - buf.WriteString("contract(") - writeIdentList(buf, x.TParams) - buf.WriteString("){...}") - // TODO(gri) fill in the rest } } diff --git a/src/go/types/object.go b/src/go/types/object.go index 21e3ddd37c..4c0613fcea 100644 --- a/src/go/types/object.go +++ b/src/go/types/object.go @@ -338,6 +338,24 @@ func (obj *Func) Scope() *Scope { return obj.typ.(*Signature).scope } func (*Func) isDependency() {} // a function may be a dependency of an initialization expression +// A Contract represents a declared contract. +type Contract struct { + object + TParams []*TypeName // type parameters in declaration order + Bounds map[*TypeName]*Named // underlying type is always *Interface +} + +// NewContract returns a new contract. +func NewContract(pos token.Pos, pkg *Package, name string) *Contract { + return &Contract{object{nil, pos, pkg, name, nil, 0, white, token.NoPos}, nil, nil} +} + +// boundsAt returns the (named) interface for the respective +// contract type parameter with the given index. +func (c *Contract) boundsAt(index int) *Named { + return c.Bounds[c.TParams[index]] +} + // A Label represents a declared label. // Labels don't have a type. type Label struct { @@ -420,6 +438,32 @@ func writeObject(buf *bytes.Buffer, obj Object, qf Qualifier) { } return + case *Contract: + buf.WriteString("contract ") + buf.WriteString(obj.name) // TODO(gri) qualify this! + buf.WriteByte('(') + for i, tpar := range obj.TParams { + if i > 0 { + buf.WriteString(", ") + } + WriteType(buf, tpar.typ, qf) + } + buf.WriteString(") {") + i := 0 + for tpar, bound := range obj.Bounds { + if i > 0 { + buf.WriteString("; ") + } + WriteType(buf, tpar.typ, qf) + buf.WriteByte(' ') + WriteType(buf, bound, qf) + buf.WriteString(" = ") + WriteType(buf, bound.underlying, qf) + i++ + } + buf.WriteByte('}') + return + case *Label: buf.WriteString("label") typ = nil diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go index 6aec2e571b..60b9b901a1 100644 --- a/src/go/types/resolver.go +++ b/src/go/types/resolver.go @@ -17,12 +17,13 @@ import ( // A declInfo describes a package-level const, type, var, or func declaration. type declInfo struct { - file *Scope // scope of file containing this declaration - lhs []*Var // lhs of n:1 variable declarations, or nil - vtyp ast.Expr // type, or nil (for const and var declarations only) - init ast.Expr // init/orig expression, or nil (for const and var declarations only) - tdecl *ast.TypeSpec // type declaration, or nil - fdecl *ast.FuncDecl // func declaration, or nil + file *Scope // scope of file containing this declaration + lhs []*Var // lhs of n:1 variable declarations, or nil + vtyp ast.Expr // type, or nil (for const and var declarations only) + init ast.Expr // init/orig expression, or nil (for const and var declarations only) + tdecl *ast.TypeSpec // type declaration, or nil + fdecl *ast.FuncDecl // func declaration, or nil + cdecl *ast.ContractSpec // contract declaration, or nil // The deps field tracks initialization expression dependencies. deps map[Object]bool // lazily initialized @@ -395,6 +396,10 @@ func (check *Checker) collectObjects() { obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Name, nil) check.declarePkgObj(s.Name, obj, &declInfo{file: fileScope, tdecl: s}) + case *ast.ContractSpec: + obj := NewContract(s.Name.Pos(), pkg, s.Name.Name) + check.declarePkgObj(s.Name, obj, &declInfo{file: fileScope, cdecl: s}) + default: check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) } diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 7f9759c3ca..0904ac6a58 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -55,9 +55,6 @@ func (check *Checker) instantiate(pos token.Pos, typ Type, targs []Type, poslist // check bounds for i, tname := range tparams { targ := targs[i] - if _, ok := targ.Underlying().(*Contract); ok { - panic("contract provided as type argument") - } tpar := tname.typ.(*TypeParam) // TODO(gri) decide if we want to keep this or standardize on the empty interface @@ -334,24 +331,7 @@ func (subst *subster) typ(typ Type) Type { subst.cache[t] = named dump(">>> subst %s(%s) with %s (new: %s)", t.underlying, subst.tpars, subst.targs, new_targs) named.underlying = subst.typ(t.underlying) - - // update all method signatures - named.methods = t.methods - /* - if len(t.methods) > 0 { - dump(">>> update method signatures:") - for _, m := range t.methods { - subst.check.objDecl(m, nil) - copy := *m - copy.typ = subst.typ(copy.typ) - dump(">>> - %s: %s -> %s", m, m.typ, copy.typ) - named.methods = append(named.methods, ©) - } - dump(">>> done with methods") - } else { - dump(">>> no methods to update") - } - */ + named.methods = t.methods // method signatures are updated lazily return named @@ -363,9 +343,6 @@ func (subst *subster) typ(typ Type) Type { } } - case *Contract: - panic("unimplemented") - default: panic("unimplemented") } diff --git a/src/go/types/testdata/contracts.go2 b/src/go/types/testdata/contracts.go2 index a3351d35b5..9614c8577f 100644 --- a/src/go/types/testdata/contracts.go2 +++ b/src/go/types/testdata/contracts.go2 @@ -10,13 +10,8 @@ contract _(A){} contract _(A, B, C){} contract _(A, B, A /* ERROR A redeclared */ ){} -// For now we also allow this form of contract declaration. -// TODO(gri) probably not a good idea. Disallow at local level for sure. -type _ contract(){} -type _ contract(A, B, A /* ERROR A redeclared */ ){} - // method constraints -contract _(A) { A } /* ERROR expected type */ +contract _(A) { A } /* ERROR expected type */ // TODO(gri) investigate this error! looks wrong contract _(A) { A m(); A add(A) int } contract _(A) { B /* ERROR B not declared by contract */ m() } contract _(A) { A m(); A m /* ERROR duplicate method */ () } @@ -46,7 +41,7 @@ contract _(A) { A B /* ERROR undeclared name: B */ } contract _(A) { A int } contract _(A) { A []int } contract _(A) { A []B /* ERROR undeclared name: B */ } -contract C(A) { A [ /* ERROR invalid type constraint */ ]C } +contract C(A) { A C /* ERROR use of contract */ } contract _(A) { A struct { f int } } contract _(A, B) { A B } @@ -56,8 +51,14 @@ contract _() { // E /* ERROR embedding not yet implemented */ } +// contract use in wrong places +const _ E /* ERROR use of contract */ = 0 +type _ E /* ERROR use of contract */ +var _ E /* ERROR use of contract */ +var _ = E /* ERROR use of contract */ + // -------------------------------------------------------------------------------------- -// Contract implementation +// Contract satisfaction // Type parameter type must be a contract. type _(type T int /* ERROR not an interface or contract */ ) struct{} diff --git a/src/go/types/type.go b/src/go/types/type.go index 682309e035..4d183fcaf6 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -523,19 +523,6 @@ func (t *Named) AddMethod(m *Func) { } } -// A Contract represents a contract. -type Contract struct { - TParams []*TypeName // type parameters in declaration order - Bounds map[*TypeName]*Named // underlying type is always *Interface -} - -// 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?) @@ -571,7 +558,6 @@ func (t *Interface) Underlying() Type { return t } func (m *Map) Underlying() Type { return m } func (c *Chan) Underlying() Type { return c } func (t *Named) Underlying() Type { return t.underlying } -func (c *Contract) Underlying() Type { return c } func (t *TypeParam) Underlying() Type { return t } // TODO(gri) should this return t.Interface() instead? func (b *Basic) String() string { return TypeString(b, nil) } @@ -585,5 +571,4 @@ func (t *Interface) String() string { return TypeString(t, nil) } func (m *Map) String() string { return TypeString(m, nil) } func (c *Chan) String() string { return TypeString(c, nil) } func (t *Named) String() string { return TypeString(t, nil) } -func (c *Contract) String() string { return TypeString(c, nil) } func (t *TypeParam) String() string { return TypeString(t, nil) } diff --git a/src/go/types/typestring.go b/src/go/types/typestring.go index 1b60c3e3f7..8ec016679f 100644 --- a/src/go/types/typestring.go +++ b/src/go/types/typestring.go @@ -270,29 +270,6 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) { // buf.WriteByte(')') // } - case *Contract: - buf.WriteString("contract(") - for i, tpar := range t.TParams { - if i > 0 { - buf.WriteString(", ") - } - writeType(buf, tpar.typ, qf, visited) - } - buf.WriteString("){") - i := 0 - for tpar, bound := range t.Bounds { - if i > 0 { - buf.WriteString("; ") - } - writeType(buf, tpar.typ, qf, visited) - buf.WriteByte(' ') - writeType(buf, bound, qf, visited) - buf.WriteString(" = ") - writeType(buf, bound.underlying, qf, visited) - i++ - } - buf.WriteByte('}') - case *TypeParam: var s string if t.obj != nil { diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 5ef84b1b27..e43dcc8211 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -98,6 +98,11 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, wantType bool) } x.mode = variable + case *Contract: + // TODO(gri) need better error message here + check.errorf(e.Pos(), "use of contract %s not in type parameter declaration", obj.name) + return + case *Func: check.addDeclDep(obj) x.mode = value @@ -374,16 +379,6 @@ func (check *Checker) typInternal(e ast.Expr, def *Named) (T Type) { typ.elem = check.typ(e.Value) return typ - case *ast.ContractType: - typ := new(Contract) - def.setUnderlying(typ) - name := "" - if def != nil { - name = def.obj.name - } - check.contractType(typ, name, e) - return typ - default: check.errorf(e.Pos(), "%s is not a type", e) }