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

@@ -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
 	}
 
+	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 x.mode == constant && isUntyped(x.typ) && (T == nil || isInterface(T)) {
+		if T == nil || isInterface(T) {
-		check.convertUntyped(x, defaultType(x.typ))
+			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 T == nil || x.isAssignableTo(check.conf, T)
-		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)
 }
 
 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.
+		if !check.assignment(x, nil) {
-		// TODO(gri) This is still not correct (_ = 1<<1e3)
+			assert(x.mode == invalid)
-		typ := x.typ
+			x.typ = nil
-		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)
+		return x.typ
-		}
-		check.updateExprType(x.expr, typ, true) // rhs has its final type
-		return typ
 	}
 
 	// If the lhs is an identifier denoting a variable v, this assignment

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)
+		T := new(Interface)
-		if x.mode == invalid {
+		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 := 0; i < nargs; i++ {
-			for i := 1; i < nargs; i++ {
+				if i > 0 {
-				arg(x, i)
+					arg(x, i) // first argument already evaluated
-				if x.mode == invalid {
+				}
+				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]

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
@@ -624,6 +632,7 @@ func Offsetof1() {
 	_ = 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(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

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 /* DISABLED duplicate case */ <<64-1:
-	case 1<<64-1:
+	case 1 /* DISABLED duplicate case */ <<64-1:
 	}
 }