go.tools/go/types: cleanup parameter passing

- now represent ...T parameter type as []T (rather than T)
- simplified call checking
- added missing check and test for x... arguments (x must be slice)

This CL will temporarily break ssa and ssa/interp.

R=adonovan, axwalk
CC=golang-dev
https://golang.org/cl/11317043

go/types/call.go

@@ -14,73 +14,70 @@ import (
 func (check *checker) call(x *operand, e *ast.CallExpr) {
 	check.exprOrType(x, e.Fun)
 	if x.mode == invalid {
-		// We don't have a valid call or conversion but we have list of arguments.
+		// We don't have a valid call or conversion but we have a list of arguments.
 		// Typecheck them independently for better partial type information in
 		// the presence of type errors.
 		for _, arg := range e.Args {
 			check.expr(x, arg)
 		}
-		goto Error
+		x.mode = invalid
+		x.expr = e
+		return
+	}
 
-	} else if x.mode == typexpr {
+	if x.mode == typexpr {
 		check.conversion(x, e, x.typ)
+		return
+	}
 
-	} else if sig, ok := x.typ.Underlying().(*Signature); ok {
+	if sig, ok := x.typ.Underlying().(*Signature); ok {
-		// check parameters
+		// function/method call
 
-		// If we have a trailing ... at the end of the parameter
-		// list, the last argument must match the parameter type
-		// []T of a variadic function parameter x ...T.
 		passSlice := false
 		if e.Ellipsis.IsValid() {
+			// last argument is of the form x...
 			if sig.isVariadic {
 				passSlice = true
 			} else {
-				check.errorf(e.Ellipsis, "cannot use ... in call to %s", e.Fun)
+				check.errorf(e.Ellipsis, "cannot use ... in call to non-variadic %s", e.Fun)
 				// ok to continue
 			}
 		}
 
-		// If we have a single argument that is a function call
+		// evaluate arguments
-		// we need to handle it separately. Determine if this
+		n := len(e.Args) // argument count
-		// is the case without checking the argument.
+		if n == 1 {
-		var call *ast.CallExpr
+			// single argument but possibly a multi-valued function call
-		if len(e.Args) == 1 {
+			arg := e.Args[0]
-			call, _ = unparen(e.Args[0]).(*ast.CallExpr)
+			check.expr(x, arg)
-		}
+			if x.mode != invalid {
-
+				if t, ok := x.typ.(*Tuple); ok {
-		n := 0 // parameter count
+					// argument is multi-valued function call
-		if call != nil {
+					n = t.Len()
-			// We have a single argument that is a function call.
+					for i := 0; i < n; i++ {
-			check.expr(x, call)
+						x.mode = value
-			if x.mode == invalid {
+						x.expr = arg
-				goto Error // TODO(gri): we can do better
+						x.typ = t.At(i).typ
-			}
+						check.argument(sig, i, x, passSlice && i == n-1)
-			if t, ok := x.typ.(*Tuple); ok {
+					}
-				// multiple result values
+				} else {
-				n = t.Len()
+					// single value
-				for i := 0; i < n; i++ {
+					check.argument(sig, 0, x, passSlice)
-					obj := t.At(i)
-					x.mode = value
-					x.expr = nil // TODO(gri) can we do better here? (for good error messages)
-					x.typ = obj.typ
-					check.argument(sig, i, nil, x, passSlice && i+1 == n)
 				}
 			} else {
-				// single result value
+				n = sig.params.Len() // avoid additional argument length errors below
-				n = 1
-				check.argument(sig, 0, nil, x, passSlice)
 			}
-
 		} else {
-			// We don't have a single argument or it is not a function call.
+			// zero or multiple arguments
-			n = len(e.Args)
 			for i, arg := range e.Args {
-				check.argument(sig, i, arg, x, passSlice && i+1 == n)
+				check.expr(x, arg)
+				if x.mode != invalid {
+					check.argument(sig, i, x, passSlice && i == n-1)
+				}
 			}
 		}
 
-		// determine if we have enough arguments
+		// check argument count
 		if sig.isVariadic {
 			// a variadic function accepts an "empty"
 			// last argument: count one extra
@@ -97,82 +94,64 @@ func (check *checker) call(x *operand, e *ast.CallExpr) {
 			x.mode = novalue
 		case 1:
 			x.mode = value
-			x.typ = sig.results.vars[0].typ
+			x.typ = sig.results.vars[0].typ // unpack tuple
 		default:
 			x.mode = value
 			x.typ = sig.results
 		}
-
+		x.expr = e
-	} else if bin, ok := x.typ.(*Builtin); ok {
+		return
-		check.builtin(x, e, bin)
-
-	} else {
-		check.invalidOp(x.pos(), "cannot call non-function %s", x)
-		goto Error
 	}
 
-	// everything went well
+	if bin, ok := x.typ.(*Builtin); ok {
-	x.expr = e
+		check.builtin(x, e, bin)
-	return
+		return
+	}
 
-Error:
+	check.invalidOp(x.pos(), "cannot call non-function %s", x)
 	x.mode = invalid
 	x.expr = e
 }
 
-// argument typechecks passing an argument arg (if arg != nil) or
+// argument checks passing of argument x to the i'th parameter of the given signature.
-// x (if arg == nil) to the i'th parameter of the given signature.
 // If passSlice is set, the argument is followed by ... in the call.
-//
+func (check *checker) argument(sig *Signature, i int, x *operand, passSlice bool) {
-func (check *checker) argument(sig *Signature, i int, arg ast.Expr, x *operand, passSlice bool) {
-	// determine parameter
-	var par *Var
 	n := sig.params.Len()
-	if i < n {
+
-		par = sig.params.vars[i]
+	// determine parameter type
-	} else if sig.isVariadic {
+	var typ Type
-		par = sig.params.vars[n-1]
+	switch {
-	} else {
+	case i < n:
-		var pos token.Pos
+		typ = sig.params.vars[i].typ
-		switch {
+	case sig.isVariadic:
-		case arg != nil:
+		typ = sig.params.vars[n-1].typ
-			pos = arg.Pos()
+		if debug {
-		case x != nil:
+			if _, ok := typ.(*Slice); !ok {
-			pos = x.pos()
+				check.dump("%s: expected slice type, got %s", sig.params.vars[n-1].Pos(), typ)
-		default:
+			}
-			// TODO(gri) what position to use?
 		}
-		check.errorf(pos, "too many arguments")
+	default:
+		check.errorf(x.pos(), "too many arguments")
 		return
 	}
 
-	// determine argument
-	var z operand
-	z.mode = variable
-	z.expr = nil // TODO(gri) can we do better here? (for good error messages)
-	z.typ = par.typ
-
-	if arg != nil {
-		check.expr(x, arg)
-	}
-	if x.mode == invalid {
-		return // ignore this argument
-	}
-
-	// check last argument of the form x...
 	if passSlice {
-		if i+1 != n {
+		// argument is of the form x...
+		if i != n-1 {
 			check.errorf(x.pos(), "can only use ... with matching parameter")
-			return // ignore this argument
+			return
 		}
-		// spec: "If the final argument is assignable to a slice type []T,
+		if _, ok := x.typ.(*Slice); !ok {
-		// it may be passed unchanged as the value for a ...T parameter if
+			check.errorf(x.pos(), "cannot use %s as parameter of type %s", x, typ)
-		// the argument is followed by ..."
+			return
-		z.typ = &Slice{elt: z.typ} // change final parameter type to []T
+		}
+	} else if sig.isVariadic && i >= n-1 {
+		// use the variadic parameter slice's element type
+		typ = typ.(*Slice).elt
 	}
 
-	if !check.assignment(x, z.typ) && x.mode != invalid {
+	if !check.assignment(x, typ) && x.mode != invalid {
-		check.errorf(x.pos(), "cannot pass argument %s to %s", x, &z)
+		check.errorf(x.pos(), "cannot pass argument %s to parameter of type %s", x, typ)
 	}
 }
 

go/types/errors.go

@@ -188,10 +188,12 @@ func writeTuple(buf *bytes.Buffer, tup *Tuple, isVariadic bool) {
 				buf.WriteString(v.name)
 				buf.WriteByte(' ')
 			}
+			typ := v.typ
 			if isVariadic && i == len(tup.vars)-1 {
 				buf.WriteString("...")
+				typ = typ.(*Slice).elt
 			}
-			writeType(buf, v.typ)
+			writeType(buf, typ)
 		}
 	}
 	buf.WriteByte(')')

go/types/gcimporter.go

@@ -535,6 +535,9 @@ func (p *gcParser) parseParameter() (par *Var, isVariadic bool) {
 		isVariadic = true
 	}
 	typ := p.parseType()
+	if isVariadic {
+		typ = &Slice{elt: typ}
+	}
 	// ignore argument tag (e.g. "noescape")
 	if p.tok == scanner.String {
 		p.next()

go/types/testdata/expr3.src

@@ -298,6 +298,7 @@ func f2(u float32, s string) {}
 func fs(s []byte) {}
 func fv(x ...int) {}
 func fi(x ... interface{}) {}
+func (T) fm(x ...int)
 
 func g0() {}
 func g1() int { return 0}
@@ -335,17 +336,44 @@ func _calls() {
 	fs /* ERROR "too few arguments" */ ()
 	fs(g0 /* ERROR "used as value" */ ())
 	fs(g1 /* ERROR "cannot pass" */ ())
-	// fs(g2()) // TODO(gri) missing position in error message
+	fs(g2 /* ERROR "cannot pass" */ /* ERROR "too many arguments" */ ())
 	fs(gs())
 
 	fv()
 	fv(1, 2.0, x)
 	fv(s /* ERROR "cannot pass" */ )
 	fv(s...)
+	fv(x /* ERROR "cannot use" */ ...)
 	fv(1, s /* ERROR "can only use ... with matching parameter" */ ...)
 	fv(gs /* ERROR "cannot pass" */ ())
 	fv(gs /* ERROR "cannot pass" */ ()...)
 
+	var t T
+	t.fm()
+	t.fm(1, 2.0, x)
+	t.fm(s /* ERROR "cannot pass" */ )
+	t.fm(g1())
+	t.fm(1, s /* ERROR "can only use ... with matching parameter" */ ...)
+	t.fm(gs /* ERROR "cannot pass" */ ())
+	t.fm(gs /* ERROR "cannot pass" */ ()...)
+
+	T.fm(t, )
+	T.fm(t, 1, 2.0, x)
+	T.fm(t, s /* ERROR "cannot pass" */ )
+	T.fm(t, g1())
+	T.fm(t, 1, s /* ERROR "can only use ... with matching parameter" */ ...)
+	T.fm(t, gs /* ERROR "cannot pass" */ ())
+	T.fm(t, gs /* ERROR "cannot pass" */ ()...)
+
+	var i interface{ fm(x ...int) } = t
+	i.fm()
+	i.fm(1, 2.0, x)
+	i.fm(s /* ERROR "cannot pass" */ )
+	i.fm(g1())
+	i.fm(1, s /* ERROR "can only use ... with matching parameter" */ ...)
+	i.fm(gs /* ERROR "cannot pass" */ ())
+	i.fm(gs /* ERROR "cannot pass" */ ()...)
+
 	fi()
 	fi(1, 2.0, x, 3.14, "foo")
 	fi(g2())

go/types/types.go

@@ -197,8 +197,18 @@ type Signature struct {
 
 // NewSignature returns a new function type for the given receiver, parameters,
 // and results, either of which may be nil. If isVariadic is set, the function
-// is variadic.
+// is variadic, it must have at least one parameter, and the last parameter
+// must be of unnamed slice type.
 func NewSignature(recv *Var, params, results *Tuple, isVariadic bool) *Signature {
+	if isVariadic {
+		n := params.Len()
+		if n == 0 {
+			panic("types.NewSignature: variadic function must have at least one parameter")
+		}
+		if _, ok := params.At(n - 1).typ.(*Slice); !ok {
+			panic("types.NewSignature: variadic parameter must be of unnamed slice type")
+		}
+	}
 	return &Signature{nil, nil, recv, params, results, isVariadic}
 }
 

go/types/typexpr.go

@@ -298,7 +298,7 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 	if list == nil {
 		return
 	}
-	var last *Var
+
 	for i, field := range list.List {
 		ftype := field.Type
 		if t, _ := ftype.(*ast.Ellipsis); t != nil {
@@ -307,37 +307,33 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 				isVariadic = true
 			} else {
 				check.invalidAST(field.Pos(), "... not permitted")
-				// ok to continue
+				// ignore ... and continue
 			}
 		}
+		typ := check.typ(ftype, nil, true)
 		// the parser ensures that f.Tag is nil and we don't
 		// care if a constructed AST contains a non-nil tag
-		typ := check.typ(ftype, nil, true)
 		if len(field.Names) > 0 {
 			// named parameter
 			for _, name := range field.Names {
 				par := NewVar(name.Pos(), check.pkg, name.Name, typ)
 				check.declare(scope, name, par)
-
+				params = append(params, par)
-				last = par
-				copy := *par
-				params = append(params, &copy)
 			}
 		} else {
 			// anonymous parameter
 			par := NewVar(ftype.Pos(), check.pkg, "", typ)
 			check.callImplicitObj(field, par)
-
-			last = nil // not accessible inside function
 			params = append(params, par)
 		}
 	}
-	// For a variadic function, change the last parameter's object type
+
-	// from T to []T (this is the type used inside the function), but
+	// For a variadic function, change the last parameter's type from T to []T.
-	// keep the params list unchanged (this is the externally visible type).
+	if isVariadic && len(params) > 0 {
-	if isVariadic && last != nil {
+		last := params[len(params)-1]
 		last.typ = &Slice{elt: last.typ}
 	}
+
 	return
 }
 
@@ -345,6 +341,7 @@ func (check *checker) collectMethods(list *ast.FieldList, cycleOk bool) (methods
 	if list == nil {
 		return nil
 	}
+
 	scope := NewScope(nil)
 	for _, f := range list.List {
 		typ := check.typ(f.Type, nil, cycleOk)
@@ -385,6 +382,7 @@ func (check *checker) collectMethods(list *ast.FieldList, cycleOk bool) (methods
 			}
 		}
 	}
+
 	return
 }