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

@@ -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

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 "<contract type>" }
+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

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)] {

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

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
 	}
 }
 

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

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)
 					}

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, &copy)
-				}
-				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")
 	}

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{}

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) }

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 {

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 := "<contract>"
-		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)
 	}