From a32c2633d024cb1869f6ee20545ddeb95f80e965 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 1 Oct 2013 18:14:15 -0700 Subject: [PATCH] go.tools/go/types: complete checking of append built-in R=adonovan CC=golang-dev https://golang.org/cl/14238043 --- go/types/api.go | 1 - go/types/builtins.go | 66 +++++++++++++++---- go/types/call.go | 114 +++++++++++++++++---------------- go/types/testdata/builtins.src | 52 +++++++++++++-- 4 files changed, 163 insertions(+), 70 deletions(-) diff --git a/go/types/api.go b/go/types/api.go index 0bde789bcd7..8e218e30a88 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -181,6 +181,5 @@ func IsAssignableTo(V, T Type) bool { return x.isAssignableTo(nil, T) // config not needed for non-constant x } -// BUG(gri): Some built-ins don't check parameters fully, yet (e.g. append). // BUG(gri): Interface vs non-interface comparisons are not correctly implemented. // BUG(gri): Switch statements don't check duplicate cases for all types for which it is required. diff --git a/go/types/builtins.go b/go/types/builtins.go index d0f806c6967..4c797a813a6 100644 --- a/go/types/builtins.go +++ b/go/types/builtins.go @@ -13,9 +13,6 @@ import ( "code.google.com/p/go.tools/go/exact" ) -// TODO(gri): Several built-ins are missing assignment checks. As a result, -// non-constant shift arguments may not be properly type-checked. - // builtin typechecks a call to a built-in and returns the result via x. // If the call has type errors, the returned x is marked as invalid. // @@ -35,7 +32,7 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) { msg = "too many" } if msg != "" { - check.invalidOp(call.Pos(), msg+" arguments for %s (expected %d, found %d)", call, bin.nargs, n) + check.invalidOp(call.Pos(), "%s arguments for %s (expected %d, found %d)", msg, call, bin.nargs, n) goto Error } @@ -57,20 +54,55 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) { switch id { case _Append: - if _, ok := x.typ.Underlying().(*Slice); !ok { - check.invalidArg(x.pos(), "%s is not a typed slice", x) + // append(s S, x ...T) S where T is the element type of S + // spec: "The variadic function append appends zero or more values x to s of type + // S, which must be a slice type, and returns the resulting slice, also of type S. + // The values x are passed to a parameter of type ...T where T is the element type + // of S and the respective parameter passing rules apply." + S := x.typ + var T Type + if s, _ := S.Underlying().(*Slice); s != nil { + T = s.elt + } else { + check.invalidArg(x.pos(), "%s is not a slice", x) goto Error } - resultTyp := x.typ - for _, arg := range args[1:] { - check.expr(x, arg) + + // remember arguments that have been evaluated already + alist := []operand{*x} + + // spec: "As a special case, append also accepts a first argument assignable + // to type []byte with a second argument of string type followed by ... . + // This form appends the bytes of the string. + if n == 2 && call.Ellipsis.IsValid() && x.isAssignableTo(check.conf, NewSlice(Typ[Byte])) { + check.expr(x, args[1]) if x.mode == invalid { goto Error } - // TODO(gri) check assignability + if isString(x.typ) { + x.mode = value + x.typ = S + x.expr = call + return + } + alist = append(alist, *x) + // fallthrough } + + // check general case by creating custom signature + sig := makeSig(S, S, NewSlice(T)) // []T required for variadic signature + sig.isVariadic = true + check.arguments(x, call, sig, func(x *operand, i int) { + // only evaluate arguments that have not been evaluated before + if i < len(alist) { + *x = alist[i] + return + } + check.expr(x, args[i]) + }) + x.mode = value - x.typ = resultTyp + x.typ = S case _Cap, _Len: mode := invalid @@ -416,6 +448,18 @@ Error: x.expr = call } +// makeSig returns the signature for the given parameter and result types. +func makeSig(result Type, params ...Type) *Signature { + list := make([]*Var, len(params)) + for i, param := range params { + list[i] = NewVar(token.NoPos, nil, "", param) + } + return &Signature{ + params: NewTuple(list...), + results: NewTuple(NewVar(token.NoPos, nil, "", result)), + } +} + // implicitArrayDeref returns A if typ is of the form *A and A is an array; // otherwise it returns typ. // diff --git a/go/types/call.go b/go/types/call.go index ebfd00de43d..80c86ce36b7 100644 --- a/go/types/call.go +++ b/go/types/call.go @@ -62,60 +62,7 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind { return statement } - 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 non-variadic %s", e.Fun) - // ok to continue - } - } - - // evaluate arguments - n := len(e.Args) // argument count - if n == 1 { - // single argument but possibly a multi-valued function call - arg := e.Args[0] - check.expr(x, arg) - if x.mode != invalid { - if t, ok := x.typ.(*Tuple); ok { - // argument is multi-valued function call - n = t.Len() - for i := 0; i < n; i++ { - x.mode = value - x.expr = arg - x.typ = t.At(i).typ - check.argument(sig, i, x, passSlice && i == n-1) - } - } else { - // single value - check.argument(sig, 0, x, passSlice) - } - } else { - n = sig.params.Len() // avoid additional argument length errors below - } - } else { - // zero or multiple arguments - for i, arg := range e.Args { - check.expr(x, arg) - if x.mode != invalid { - check.argument(sig, i, x, passSlice && i == n-1) - } - } - } - - // check argument count - if sig.isVariadic { - // a variadic function accepts an "empty" - // last argument: count one extra - n++ - } - if n < sig.params.Len() { - check.errorf(e.Rparen, "too few arguments in call to %s", e.Fun) - // ok to continue - } + check.arguments(x, e, sig, func(x *operand, i int) { check.expr(x, e.Args[i]) }) // determine result switch sig.results.Len() { @@ -134,6 +81,65 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind { } } +// arguments checks argument passing for the call with the given signature. +// The arg function provides the operand for the i'th argument. +func (check *checker) arguments(x *operand, call *ast.CallExpr, sig *Signature, arg func(*operand, int)) { + passSlice := false + if call.Ellipsis.IsValid() { + // last argument is of the form x... + if sig.isVariadic { + passSlice = true + } else { + check.errorf(call.Ellipsis, "cannot use ... in call to non-variadic %s", call.Fun) + // ok to continue + } + } + + // evaluate arguments + n := len(call.Args) // argument count + if n == 1 { + // single argument but possibly a multi-valued function call + arg(x, 0) + if x.mode != invalid { + if t, ok := x.typ.(*Tuple); ok { + // argument is multi-valued function call + n = t.Len() + expr := call.Args[0] + for i := 0; i < n; i++ { + x.mode = value + x.expr = expr + x.typ = t.At(i).typ + check.argument(sig, i, x, passSlice && i == n-1) + } + } else { + // single value + check.argument(sig, 0, x, passSlice) + } + } else { + n = sig.params.Len() // avoid additional argument length errors below + } + } else { + // zero or multiple arguments + for i := range call.Args { + arg(x, i) + if x.mode != invalid { + check.argument(sig, i, x, passSlice && i == n-1) + } + } + } + + // check argument count + if sig.isVariadic { + // a variadic function accepts an "empty" + // last argument: count one extra + n++ + } + if n < sig.params.Len() { + check.errorf(call.Rparen, "too few arguments in call to %s", call.Fun) + // ok to continue + } +} + // argument checks passing of argument x 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) { diff --git a/go/types/testdata/builtins.src b/go/types/testdata/builtins.src index 8dcfdf8e97e..3ce6f985460 100644 --- a/go/types/testdata/builtins.src +++ b/go/types/testdata/builtins.src @@ -8,15 +8,59 @@ package builtins import "unsafe" -func _append() { +func _append1() { + var b byte var x int var s []byte _ = append /* ERROR "argument" */ () - _ = append("foo" /* ERROR "not a typed slice" */) - _ = append(nil /* ERROR "not a typed slice" */, s) - _ = append(x /* ERROR "not a typed slice" */, s) + _ = append("foo" /* ERROR "not a slice" */ ) + _ = append(nil /* ERROR "not a slice" */ , s) + _ = append(x /* ERROR "not a slice" */ , s) _ = append(s) append /* ERROR "not used" */ (s) + + _ = append(s, b) + _ = append(s, x /* ERROR cannot pass argument x */ ) + _ = append(s, s /* ERROR cannot pass argument s */ ) + _ = append(s /* ERROR can only use ... with matching parameter */ ...) + _ = append(s, b, s /* ERROR can only use ... with matching parameter */ ...) + _ = append(s, 1, 2, 3) + _ = append(s, 1, 2, 3, x /* ERROR cannot pass argument x */ , 5, 6, 6) + _ = append(s, 1, 2, s /* ERROR can only use ... with matching parameter */ ...) + _ = append([]interface{}(nil), 1, 2, "foo", x, 3.1425, false) + + type S []byte + type T string + var t T + _ = append(s, "foo" /* ERROR cannot convert */ ) + _ = append(s, "foo"...) + _ = append(S(s), "foo" /* ERROR cannot convert */ ) + _ = append(S(s), "foo"...) + _ = append(s, t /* ERROR cannot pass argument t */ ) + _ = append(s, t...) + _ = append(s, T("foo")...) + _ = append(S(s), t /* ERROR cannot pass argument t */ ) + _ = append(S(s), t...) + _ = append(S(s), T("foo")...) + _ = append([]string{}, t /* ERROR cannot pass argument t */ , "foo") + _ = append([]T{}, t, "foo") +} + +// from the spec +func _append2() { + s0 := []int{0, 0} + s1 := append(s0, 2) // append a single element s1 == []int{0, 0, 2} + s2 := append(s1, 3, 5, 7) // append multiple elements s2 == []int{0, 0, 2, 3, 5, 7} + s3 := append(s2, s0...) // append a slice s3 == []int{0, 0, 2, 3, 5, 7, 0, 0} + s4 := append(s3[3:6], s3[2:]...) // append overlapping slice s4 == []int{3, 5, 7, 2, 3, 5, 7, 0, 0} + + var t []interface{} + t = append(t, 42, 3.1415, "foo") // t == []interface{}{42, 3.1415, "foo"} + + var b []byte + b = append(b, "bar"...) // append string contents b == []byte{'b', 'a', 'r' } + + _ = s4 } func _cap() {