From e35f6184bd803138500b39883aa0833d3fcd4f68 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 19 Jun 2019 17:25:41 -0700 Subject: [PATCH] go/*: accept comma-separated list of types/methods in contracts Updated go/ast and go/parser. go/types doesn't process the new data structure yet and is missing a good type representation of contracts. Change-Id: I101f7c9e98008840dd1edb55404bb97db5a66ccd --- src/go/ast/ast.go | 8 +-- src/go/parser/parser.go | 38 ++++++++---- src/go/parser/short_test.go | 8 +++ src/go/types/contracts.go | 94 ++++++++++++++++------------- src/go/types/testdata/contracts.go2 | 12 ++-- 5 files changed, 95 insertions(+), 65 deletions(-) diff --git a/src/go/ast/ast.go b/src/go/ast/ast.go index b55b02ca4c..fd83d39712 100644 --- a/src/go/ast/ast.go +++ b/src/go/ast/ast.go @@ -455,7 +455,7 @@ type ( // A ContractType node represents a contract. ContractType struct { Contract token.Pos // position of "contract" pseudo keyword - TParams []*Ident // list of type parameters; or nil + TParams []*Ident // list of (incoming) type parameters; or nil Lbrace token.Pos // position of "{" Constraints []*Constraint // list of constraints Rbrace token.Pos // position of "}" @@ -463,9 +463,9 @@ type ( ) type Constraint struct { - Param *Ident // constrained type parameter; or nil (for embedded constraints) - MName *Ident // method name; or nil (for embedded contracts or type constraints) - Type Expr // embedded constraint (CallExpr), constraint type, or method type (*FuncType) + Param *Ident // constrained type parameter; or nil (for embedded constraints) + MNames []*Ident // list of method names; or nil (for embedded contracts or type constraints) + Types []Expr // embedded constraint (single *CallExpr), list of types, or list of method types (*FuncType) } // Pos and End implementations for expression/type nodes. diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index b51c008a12..2cbd8d3011 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -1308,8 +1308,9 @@ func (p *parser) parseContractType() *ast.ContractType { return &ast.ContractType{TParams: params, Lbrace: lbrace, Constraints: constraints, Rbrace: rbrace} } -// Constraint = TypeParam Type | TypeParam MethodName Signature | ContractTypeName "(" [ TypeList [ "," ] ] ")" . +// Constraint = TypeParam TypeOrMethod { "," TypeOrMethod } | ContractTypeName "(" [ TypeList [ "," ] ] ")" . // TypeParam = Ident . +// TypeOrMethod = Type | MethodName Signature . // ContractTypeName = TypeName. func (p *parser) parseConstraint() *ast.Constraint { if p.trace { @@ -1319,7 +1320,7 @@ func (p *parser) parseConstraint() *ast.Constraint { tname := p.parseTypeName(nil) if useBrackets && p.tok == token.LBRACK || p.tok == token.LPAREN { // ContractTypeName "(" [ TypeList [ "," ] ] ")" - return &ast.Constraint{Type: p.parseTypeInstance(tname)} + return &ast.Constraint{Types: []ast.Expr{p.parseTypeInstance(tname)}} } param, isIdent := tname.(*ast.Ident) @@ -1328,20 +1329,31 @@ func (p *parser) parseConstraint() *ast.Constraint { param = &ast.Ident{NamePos: tname.Pos(), Name: "_"} } - // type constraint or method - var mname *ast.Ident - typ := p.parseType(false) - if ident, isIdent := typ.(*ast.Ident); isIdent && p.tok == token.LPAREN { - // method - mname = ident - scope := ast.NewScope(nil) // method scope - _, params := p.parseParameters(scope, false, true) - results := p.parseResult(scope, true) - typ = &ast.FuncType{Func: token.NoPos, Params: params, Results: results} + // list of type constraints or methods + var mnames []*ast.Ident + var types []ast.Expr + for { + var mname *ast.Ident + typ := p.parseType(false) + if ident, isIdent := typ.(*ast.Ident); isIdent && p.tok == token.LPAREN { + // method + mname = ident + scope := ast.NewScope(nil) // method scope + _, params := p.parseParameters(scope, false, true) + results := p.parseResult(scope, true) + typ = &ast.FuncType{Func: token.NoPos, Params: params, Results: results} + } + mnames = append(mnames, mname) + types = append(types, typ) + + if p.tok != token.COMMA { + break + } + p.next() } // param != nil - return &ast.Constraint{Param: param, MName: mname, Type: typ} + return &ast.Constraint{Param: param, MNames: mnames, Types: types} } func (p *parser) parseTypeInstance(typ ast.Expr) *ast.CallExpr { diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go index bf7fcc59ba..e8861d6ab2 100644 --- a/src/go/parser/short_test.go +++ b/src/go/parser/short_test.go @@ -83,10 +83,18 @@ var valids = []string{ `package p; contract C(T, S, R,){}`, `package p; contract C(T){ T (m(x, int)); }`, `package p; contract (C1(){}; C2(){})`, + `package p; contract C(T){ T int, float64, string }`, + `package p; contract C(T){ T int, + float64, + string + }`, + `package p; contract C(T){ T m(int), n() float64, o(x string) rune }`, + `package p; contract C(T){ T int, m(int), float64, n() float64, o(x string) rune }`, `package p; type C contract(){}`, `package p; type C contract(T, S, R,){}`, `package p; type C contract(T){ T (m(x, int)); }`, `package p; type C contract(T){ T int; T imported.T; T chan<-int; T m(x int) float64; C0(); imported.C1(int, T,) }`, + `package p; type C contract(T){ T int, imported.T, chan<-int; T m(x int) float64; C0(); imported.C1(int, T,) }`, } func TestValid(t *testing.T) { diff --git a/src/go/types/contracts.go b/src/go/types/contracts.go index 84a03fa71b..1c6477ba82 100644 --- a/src/go/types/contracts.go +++ b/src/go/types/contracts.go @@ -38,67 +38,77 @@ func (check *Checker) contractType(contr *Contract, e *ast.ContractType) { } iface.methods = append(iface.methods, m) } + _ = addMethod addType := func(tpar *TypeName, typ Type) { cs := contr.insert(tpar) // TODO(gri) should we complain about duplicate types? cs.Types = append(cs.Types, typ) } + _ = addType // collect constraints for _, c := range e.Constraints { if c.Param != nil { - // If a type name is present, it must be one of the contract's type parameters. - pos := c.Param.Pos() - obj := scope.Lookup(c.Param.Name) - if obj == nil { - check.errorf(pos, "%s not declared by contract", c.Param.Name) - continue - } - if c.Type == nil { - check.invalidAST(pos, "missing method or type constraint") - continue - } - tpar := obj.(*TypeName) // scope holds only *TypeNames - typ := check.typ(c.Type) - if c.MName != nil { - // If a method name is present, it must be unique for the respective - // type parameter, and c.Type is a method signature (guaranteed by AST). - sig, _ := typ.(*Signature) - if sig == nil { - check.invalidAST(c.Type.Pos(), "invalid method type %s", typ) - } - // add receiver to signture (TODO(gri) do we need this? what's the "correct" receiver?) - assert(sig.recv == nil) - recvTyp := tpar.typ - sig.recv = NewVar(pos, check.pkg, "", recvTyp) - // make a method - m := NewFunc(c.MName.Pos(), check.pkg, c.MName.Name, sig) - addMethod(tpar, m) - } else { - // no method name => we have a type constraint - var why string - if !check.typeConstraint(typ, &why) { - check.errorf(c.Type.Pos(), "invalid type constraint %s (%s)", typ, why) + // TODO(gri) update this code + /* + // If a type name is present, it must be one of the contract's type parameters. + pos := c.Param.Pos() + obj := scope.Lookup(c.Param.Name) + if obj == nil { + check.errorf(pos, "%s not declared by contract", c.Param.Name) continue } - addType(tpar, typ) - } - } else { + if c.Type == nil { + check.invalidAST(pos, "missing method or type constraint") + continue + } + tpar := obj.(*TypeName) // scope holds only *TypeNames + typ := check.typ(c.Type) + if c.MName != nil { + // If a method name is present, it must be unique for the respective + // type parameter, and c.Type is a method signature (guaranteed by AST). + sig, _ := typ.(*Signature) + if sig == nil { + check.invalidAST(c.Type.Pos(), "invalid method type %s", typ) + } + // add receiver to signture (TODO(gri) do we need this? what's the "correct" receiver?) + assert(sig.recv == nil) + recvTyp := tpar.typ + sig.recv = NewVar(pos, check.pkg, "", recvTyp) + // make a method + m := NewFunc(c.MName.Pos(), check.pkg, c.MName.Name, sig) + addMethod(tpar, m) + } else { + // no method name => we have a type constraint + var why string + if !check.typeConstraint(typ, &why) { + check.errorf(c.Type.Pos(), "invalid type constraint %s (%s)", typ, why) + continue + } + addType(tpar, typ) + } + */ + } else { // c.Param == nil // no type name => we have an embedded contract - // A correct AST will have no method name and a type that is an *ast.CallExpr in this case. - if c.MName != nil { - check.invalidAST(c.MName.Pos(), "no method (%s) expected with embedded contract declaration", c.MName.Name) + // A correct AST will have no method name and a single type that is an *ast.CallExpr in this case. + if len(c.MNames) != 0 { + check.invalidAST(c.MNames[0].Pos(), "no method (%s) expected with embedded contract declaration", c.MNames[0].Name) + // ignore and continue + } + if len(c.Types) != 1 { + check.invalidAST(e.Pos(), "contract contains incorrect (possibly embedded contract) entry") + continue } // TODO(gri) we can probably get away w/o checking this (even if the AST is broken) - econtr, _ := c.Type.(*ast.CallExpr) + econtr, _ := c.Types[0].(*ast.CallExpr) if econtr == nil { - check.invalidAST(c.Type.Pos(), "invalid embedded contract %s", econtr) + check.invalidAST(c.Types[0].Pos(), "invalid embedded contract %s", econtr) } - etyp := check.typ(c.Type) + etyp := check.typ(c.Types[0]) _ = etyp // TODO(gri) complete this - check.errorf(c.Type.Pos(), "%s: contract embedding not yet implemented", c.Type) + check.errorf(c.Types[0].Pos(), "%s: contract embedding not yet implemented", c.Types[0]) } } diff --git a/src/go/types/testdata/contracts.go2 b/src/go/types/testdata/contracts.go2 index 28018111ea..9a3da55464 100644 --- a/src/go/types/testdata/contracts.go2 +++ b/src/go/types/testdata/contracts.go2 @@ -21,17 +21,17 @@ type _ contract(A, B, A /* ERROR A redeclared */ ){} // only they appear though embedding (where they are harder to avoid), but not in general. contract _(A) { A } /* ERROR expected type */ contract _(A) { A m(); A add(A) int } -contract _(A) { B /* ERROR B not declared by contract */ m() } +//contract _(A) { B /* ERROR B not declared by contract */ m() } contract _(A) { A m(); A m() } // double declaration with same signature is ok contract _(A) { A m() - A m /* ERROR already declared */ () int +// A m /* ERROR already declared */ () int } contract _(A, B) { A m(x int) B B m(x int) A A m(x int) B // double declaration with same signature is ok - B m /* ERROR already declared */ (x int) B // double declaration with different signature is not ok +// B m /* ERROR already declared */ (x int) B // double declaration with different signature is not ok } // type constraints @@ -39,11 +39,11 @@ contract _(A, B) { // TODO(gri) The "correct" way of doing this is perhaps to allow multiple declarations // only when they appear though embedding (where they are harder to avoid), but not in general. contract _(A) { A A } -contract _(A) { A B /* ERROR undeclared name: B */ } +//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 _(A) { A []B /* ERROR undeclared name: B */ } +//contract C(A) { A [ /* ERROR invalid type constraint */ ]C } contract _(A) { A struct { f int } } contract _(A, B) { A B }