From bbf45e5e0c89101f7115662c03799ee239b19a12 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 26 Nov 2013 09:34:12 -0800 Subject: [PATCH] go.tools/go/types: missing assignment checks when assigning to _ R=adonovan, gri CC=golang-dev https://golang.org/cl/31340044 --- go/types/assignments.go | 54 +++++++++++++--------------------- go/types/builtins.go | 31 ++++++++++++------- go/types/testdata/builtins.src | 22 ++++++++++---- go/types/testdata/stmt0.src | 7 ++--- 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/go/types/assignments.go b/go/types/assignments.go index 9a9a6123a4f..af875a00682 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -20,7 +20,6 @@ import ( // Use T == nil to indicate assignment to an untyped blank identifier. // // TODO(gri) Should find a better way to handle in-band errors. -// TODO(gri) The T == nil mechanism is not yet used - should simplify callers eventually. // func (check *checker) assignment(x *operand, T Type) bool { switch x.mode { @@ -41,13 +40,22 @@ func (check *checker) assignment(x *operand, T Type) bool { return false } - // spec: "If an untyped constant is assigned to a variable of interface - // type or the blank identifier, the constant is first converted to type - // bool, rune, int, float64, complex128 or string respectively, depending - // on whether the value is a boolean, rune, integer, floating-point, complex, - // or string constant." - if x.mode == constant && isUntyped(x.typ) && (T == nil || isInterface(T)) { - check.convertUntyped(x, defaultType(x.typ)) + if isUntyped(x.typ) { + target := T + // spec: "If an untyped constant is assigned to a variable of interface + // type or the blank identifier, the constant is first converted to type + // bool, rune, int, float64, complex128 or string respectively, depending + // on whether the value is a boolean, rune, integer, floating-point, complex, + // or string constant." + if T == nil || isInterface(T) { + if T == nil && x.typ == Typ[UntypedNil] { + check.errorf(x.pos(), "use of untyped nil") + x.mode = invalid + return false + } + target = defaultType(x.typ) + } + check.convertUntyped(x, target) if x.mode == invalid { return false } @@ -56,19 +64,7 @@ func (check *checker) assignment(x *operand, T Type) bool { // spec: "If a left-hand side is the blank identifier, any typed or // non-constant value except for the predeclared identifier nil may // be assigned to it." - if T == nil && (x.mode != constant || isTyped(x.typ)) && !x.isNil() { - return true - } - - // If we still have an untyped x, try to convert it to T. - if isUntyped(x.typ) { - check.convertUntyped(x, T) - if x.mode == invalid { - return false - } - } - - return x.isAssignableTo(check.conf, T) + return T == nil || x.isAssignableTo(check.conf, T) } func (check *checker) initConst(lhs *Const, x *operand) { @@ -147,19 +143,11 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type { // Don't evaluate lhs if it is the blank identifier. if ident != nil && ident.Name == "_" { check.recordObject(ident, nil) - // If the lhs is untyped, determine the default type. - // TODO(gri) This is still not correct (_ = 1<<1e3) - typ := x.typ - if isUntyped(typ) { - // convert untyped types to default types - if typ == Typ[UntypedNil] { - check.errorf(x.pos(), "use of untyped nil") - return nil // nothing else to check - } - typ = defaultType(typ) + if !check.assignment(x, nil) { + assert(x.mode == invalid) + x.typ = nil } - check.updateExprType(x.expr, typ, true) // rhs has its final type - return typ + return x.typ } // If the lhs is an identifier denoting a variable v, this assignment diff --git a/go/types/builtins.go b/go/types/builtins.go index cacc2c94d4a..be10b801f37 100644 --- a/go/types/builtins.go +++ b/go/types/builtins.go @@ -408,15 +408,15 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b case _Panic: // panic(x) - arg(x, 0) - if x.mode == invalid { + T := new(Interface) + if !check.assignment(x, T) { + assert(x.mode == invalid) return } - // TODO(gri) arguments must be assignable to _ x.mode = novalue if check.Types != nil { - check.recordBuiltinType(call.Fun, makeSig(nil, new(Interface))) + check.recordBuiltinType(call.Fun, makeSig(nil, T)) } case _Print, _Println: @@ -425,14 +425,15 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b var params []Type if nargs > 0 { params = make([]Type, nargs) - params[0] = x.typ // first argument already evaluated - for i := 1; i < nargs; i++ { - arg(x, i) - if x.mode == invalid { + for i := 0; i < nargs; i++ { + if i > 0 { + arg(x, i) // first argument already evaluated + } + if !check.assignment(x, nil) { + assert(x.mode == invalid) return } params[i] = x.typ - // TODO(gri) arguments must be assignable to _ } } @@ -451,7 +452,11 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b case _Alignof: // unsafe.Alignof(x T) uintptr - // TODO(gri) argument must be assignable to _ + if !check.assignment(x, nil) { + assert(x.mode == invalid) + return + } + x.mode = constant x.val = exact.MakeInt64(check.conf.alignof(x.typ)) x.typ = Typ[Uintptr] @@ -498,7 +503,11 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b case _Sizeof: // unsafe.Sizeof(x T) uintptr - // TODO(gri) argument must be assignable to _ + if !check.assignment(x, nil) { + assert(x.mode == invalid) + return + } + x.mode = constant x.val = exact.MakeInt64(check.conf.sizeof(x.typ)) x.typ = Typ[Uintptr] diff --git a/go/types/testdata/builtins.src b/go/types/testdata/builtins.src index 65fa1d79731..0ecad64008b 100644 --- a/go/types/testdata/builtins.src +++ b/go/types/testdata/builtins.src @@ -443,7 +443,8 @@ func panic1() { panic(0) panic("foo") panic(false) - panic(1<<1000) // TODO(gri) argument should be assignable to _ + panic(1<<10) + panic(1 /* ERROR overflows */ <<1000) _ = panic /* ERROR used as value */ (0) var s []byte @@ -466,7 +467,9 @@ func print1() { print("foo") print(2.718281828) print(false) - print(1<<1000, 1<<1000) // TODO(gri) arguments should be assignable to _ + print(1<<10) + print(1 /* ERROR overflows */ <<1000) + println(nil /* ERROR untyped nil */ ) var s []int print(s... /* ERROR invalid use of \.\.\. */ ) @@ -490,7 +493,9 @@ func println1() { println("foo") println(2.718281828) println(false) - println(1<<1000, 1<<1000) // TODO(gri) arguments should be assignable to _ + println(1<<10) + println(1 /* ERROR overflows */ <<1000) + println(nil /* ERROR untyped nil */ ) var s []int println(s... /* ERROR invalid use of \.\.\. */ ) @@ -596,6 +601,9 @@ func Alignof1() { _ = unsafe.Alignof(int /* ERROR not an expression */) _ = unsafe.Alignof(42) _ = unsafe.Alignof(new(struct{})) + _ = unsafe.Alignof(1<<10) + _ = unsafe.Alignof(1 /* ERROR overflows */ <<1000) + _ = unsafe.Alignof(nil /* ERROR "untyped nil */ ) unsafe /* ERROR not used */ .Alignof(x) var y S0 @@ -622,8 +630,9 @@ func Offsetof1() { var x struct{ f int } _ = unsafe.Offsetof() // ERROR not enough arguments _ = unsafe.Offsetof(1, 2) // ERROR too many arguments - _ = unsafe.Offsetof(int /* ERROR not a selector expression */) - _ = unsafe.Offsetof(x /* ERROR not a selector expression */) + _ = unsafe.Offsetof(int /* ERROR not a selector expression */ ) + _ = unsafe.Offsetof(x /* ERROR not a selector expression */ ) + _ = unsafe.Offsetof(nil /* ERROR not a selector expression */ ) _ = unsafe.Offsetof(x.f) _ = unsafe.Offsetof((x.f)) _ = unsafe.Offsetof((((((((x))).f))))) @@ -680,6 +689,9 @@ func Sizeof1() { _ = unsafe.Sizeof(int /* ERROR not an expression */) _ = unsafe.Sizeof(42) _ = unsafe.Sizeof(new(complex128)) + _ = unsafe.Sizeof(1<<10) + _ = unsafe.Sizeof(1 /* ERROR overflows */ <<1000) + _ = unsafe.Sizeof(nil /* ERROR untyped nil */ ) unsafe /* ERROR not used */ .Sizeof(x) // basic types have size guarantees diff --git a/go/types/testdata/stmt0.src b/go/types/testdata/stmt0.src index 12bb2d9b42c..c70ef3e5225 100644 --- a/go/types/testdata/stmt0.src +++ b/go/types/testdata/stmt0.src @@ -90,7 +90,7 @@ func assignments1() { // assignments to _ _ = nil /* ERROR "use of untyped nil" */ - _ = 1<<1000 // TODO(gri) this should fail + _ = 1 /* ERROR overflow */ <<1000 (_) = 0 } @@ -388,10 +388,9 @@ func switches0() { case 1 /* DISABLED "duplicate case" */ : } - // TODO(gri) duplicate 64bit values that don't fit into an int64 are not yet detected switch uint64(x) { - case 1<<64-1: - case 1<<64-1: + case 1 /* DISABLED duplicate case */ <<64-1: + case 1 /* DISABLED duplicate case */ <<64-1: } }