go.tools/go/types: various cleanups

- use unpack also for assignments
- run benchmarks only requested (-b flag)

R=adonovan
CC=golang-dev
https://golang.org/cl/31190044

go/types/assignments.go

@@ -13,7 +13,7 @@ import (
 	"code.google.com/p/go.tools/go/exact"
 )
 
-// assignment reports whether x can be assigned to a variable of type 'T',
+// assignment reports whether x can be assigned to a variable of type T,
 // if necessary by attempting to convert untyped values to the appropriate
 // type. If x.mode == invalid upon return, then assignment has already
 // issued an error message and the caller doesn't have to report another.
@@ -148,8 +148,6 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type {
 	if ident != nil && ident.Name == "_" {
 		check.recordObject(ident, nil)
 		// If the lhs is untyped, determine the default type.
-		// The spec is unclear about this, but gc appears to
-		// do this.
 		// TODO(gri) This is still not correct (_ = 1<<1e3)
 		typ := x.typ
 		if isUntyped(typ) {
@@ -188,8 +186,8 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type {
 		return nil
 	}
 
-	// spec: Each left-hand side operand must be addressable, a map index
+	// spec: "Each left-hand side operand must be addressable, a map index
-	// expression, or the blank identifier. Operands may be parenthesized.
+	// expression, or the blank identifier. Operands may be parenthesized."
 	switch z.mode {
 	case invalid:
 		return nil
@@ -214,137 +212,57 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type {
 // return expressions, and returnPos is the position of the return statement.
 func (check *checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos) {
 	l := len(lhs)
-	r := len(rhs)
+	get, r, commaOk := unpack(func(x *operand, i int) { check.expr(x, rhs[i]) }, len(rhs), l == 2 && !returnPos.IsValid())
-	assert(l > 0)
+	if l != r {
-
+		invalidateVars(lhs)
-	// If the lhs and rhs have corresponding expressions,
+		if returnPos.IsValid() {
-	// treat each matching pair as an individual pair.
+			check.errorf(returnPos, "wrong number of return values (want %d, got %d)", l, r)
-	if l == r {
+			return
-		var x operand
-		for i, e := range rhs {
-			check.expr(&x, e)
-			check.initVar(lhs[i], &x)
 		}
+		check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r)
 		return
 	}
 
-	// Otherwise, the rhs must be a single expression (possibly
+	var x operand
-	// a function call returning multiple values, or a comma-ok
+	if commaOk {
-	// expression).
+		var a [2]Type
-	if r == 1 {
+		for i := range a {
-		// l > 1
+			get(&x, i)
-		// Start with rhs so we have expression types
+			a[i] = check.initVar(lhs[i], &x)
-		// for declarations with implicit types.
-		var x operand
-		rhs := rhs[0]
-		check.expr(&x, rhs)
-		if x.mode == invalid {
-			invalidateVars(lhs)
-			return
 		}
-
+		check.recordCommaOkTypes(rhs[0], a)
-		if t, ok := x.typ.(*Tuple); ok {
-			// function result
-			r = t.Len()
-			if l == r {
-				for i, lhs := range lhs {
-					x.mode = value
-					x.expr = rhs
-					x.typ = t.At(i).typ
-					check.initVar(lhs, &x)
-				}
-				return
-			}
-		}
-
-		if !returnPos.IsValid() && (x.mode == mapindex || x.mode == commaok) && l == 2 {
-			// comma-ok expression (not permitted with return statements)
-			x.mode = value
-			t1 := check.initVar(lhs[0], &x)
-
-			x.mode = value
-			x.expr = rhs
-			x.typ = Typ[UntypedBool]
-			t2 := check.initVar(lhs[1], &x)
-
-			if t1 != nil && t2 != nil {
-				check.recordCommaOkTypes(rhs, t1, t2)
-			}
-			return
-		}
-	}
-
-	invalidateVars(lhs)
-
-	// lhs variables may be function result parameters (return statement);
-	// use rhs position for properly located error messages
-	if returnPos.IsValid() {
-		check.errorf(returnPos, "wrong number of return values (want %d, got %d)", l, r)
 		return
 	}
-	check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r)
+
+	for i, lhs := range lhs {
+		get(&x, i)
+		check.initVar(lhs, &x)
+	}
 }
 
 func (check *checker) assignVars(lhs, rhs []ast.Expr) {
 	l := len(lhs)
-	r := len(rhs)
+	get, r, commaOk := unpack(func(x *operand, i int) { check.expr(x, rhs[i]) }, len(rhs), l == 2)
-	assert(l > 0)
+	if l != r {
-
+		check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r)
-	// If the lhs and rhs have corresponding expressions,
-	// treat each matching pair as an individual pair.
-	if l == r {
-		var x operand
-		for i, e := range rhs {
-			check.expr(&x, e)
-			check.assignVar(lhs[i], &x)
-		}
 		return
 	}
 
-	// Otherwise, the rhs must be a single expression (possibly
+	var x operand
-	// a function call returning multiple values, or a comma-ok
+	if commaOk {
-	// expression).
+		var a [2]Type
-	if r == 1 {
+		for i := range a {
-		// l > 1
+			get(&x, i)
-		var x operand
+			a[i] = check.assignVar(lhs[i], &x)
-		rhs := rhs[0]
-		check.expr(&x, rhs)
-		if x.mode == invalid {
-			return
-		}
-
-		if t, ok := x.typ.(*Tuple); ok {
-			// function result
-			r = t.Len()
-			if l == r {
-				for i, lhs := range lhs {
-					x.mode = value
-					x.expr = rhs
-					x.typ = t.At(i).typ
-					check.assignVar(lhs, &x)
-				}
-				return
-			}
-		}
-
-		if (x.mode == mapindex || x.mode == commaok) && l == 2 {
-			// comma-ok expression
-			x.mode = value
-			t1 := check.assignVar(lhs[0], &x)
-
-			x.mode = value
-			x.expr = rhs
-			x.typ = Typ[UntypedBool]
-			t2 := check.assignVar(lhs[1], &x)
-
-			if t1 != nil && t2 != nil {
-				check.recordCommaOkTypes(rhs, t1, t2)
-			}
-			return
 		}
+		check.recordCommaOkTypes(rhs[0], a)
+		return
 	}
 
-	check.errorf(rhs[0].Pos(), "assignment count mismatch (%d vs %d)", l, r)
+	for i, lhs := range lhs {
+		get(&x, i)
+		check.assignVar(lhs, &x)
+	}
 }
 
 func (check *checker) shortVarDecl(pos token.Pos, lhs, rhs []ast.Expr) {

go/types/builtins.go

@@ -33,7 +33,7 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 	switch id {
 	default:
 		// make argument getter
-		arg, nargs = unpack(func(x *operand, i int) { check.expr(x, call.Args[i]) }, nargs, false)
+		arg, nargs, _ = unpack(func(x *operand, i int) { check.expr(x, call.Args[i]) }, nargs, false)
 		// evaluate first argument, if present
 		if nargs > 0 {
 			arg(x, 0)

go/types/call.go

@@ -60,7 +60,7 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind {
 			return statement
 		}
 
-		arg, n := unpack(func(x *operand, i int) { check.expr(x, e.Args[i]) }, len(e.Args), false)
+		arg, n, _ := unpack(func(x *operand, i int) { check.expr(x, e.Args[i]) }, len(e.Args), false)
 		check.arguments(x, e, sig, arg, n)
 
 		// determine result
@@ -90,8 +90,6 @@ func (check *checker) use(list []ast.Expr) {
 	}
 }
 
-// TODO(gri) use unpack for assignment checking as well.
-
 // A getter sets x as the i'th operand, where 0 <= i < n and n is the total
 // number of operands (context-specific, and maintained elsewhere). A getter
 // type-checks the i'th operand; the details of the actual check are getter-
@@ -101,17 +99,20 @@ type getter func(x *operand, i int)
 // unpack takes a getter get and a number of operands n. If n == 1 and the
 // first operand is a function call, or a comma,ok expression and allowCommaOk
 // is set, the result is a new getter and operand count providing access to the
-// function results, or comma,ok values, respectively. In all other cases, the
+// function results, or comma,ok values, respectively. The third result value
-// incoming getter and operand count are returned unchanged. In other words,
+// reports if it is indeed the comma,ok case. In all other cases, the incoming
-// if there's exactly one operand that - after type-checking by calling get -
+// getter and operand count are returned unchanged, and the third result value
-// stands for multiple operands, the resulting getter provides access to those
+// is false.
-// operands instead.
+//
+// In other words, if there's exactly one operand that - after type-checking by
+// calling get - stands for multiple operands, the resulting getter provides access
+// to those operands instead.
 //
 // Note that unpack may call get(..., 0); but if the result getter is called
 // at most once for a given operand index i (including i == 0), that operand
 // is guaranteed to cause only one call of the incoming getter with that i.
 //
-func unpack(get getter, n int, allowCommaOk bool) (getter, int) {
+func unpack(get getter, n int, allowCommaOk bool) (getter, int, bool) {
 	if n == 1 {
 		// possibly result of an n-valued function call or comma,ok value
 		var x0 operand
@@ -121,9 +122,8 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int) {
 				if i != 0 {
 					unreachable()
 				}
-				// i == 0
 				x.mode = invalid
-			}, 1
+			}, 1, false
 		}
 
 		if t, ok := x0.typ.(*Tuple); ok {
@@ -132,26 +132,18 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int) {
 				x.mode = value
 				x.expr = x0.expr
 				x.typ = t.At(i).typ
-			}, t.Len()
+			}, t.Len(), false
 		}
 
 		if x0.mode == mapindex || x0.mode == commaok {
 			// comma-ok value
 			if allowCommaOk {
+				a := [2]Type{x0.typ, Typ[UntypedBool]}
 				return func(x *operand, i int) {
-					switch i {
+					x.mode = value
-					case 0:
+					x.expr = x0.expr
-						x.mode = value
+					x.typ = a[i]
-						x.expr = x0.expr
+				}, 2, true
-						x.typ = x0.typ
-					case 1:
-						x.mode = value
-						x.expr = x0.expr
-						x.typ = Typ[UntypedBool]
-					default:
-						unreachable()
-					}
-				}, 2
 			}
 			x0.mode = value
 		}
@@ -162,16 +154,16 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int) {
 				unreachable()
 			}
 			*x = x0
-		}, 1
+		}, 1, false
 	}
 
 	// zero or multiple values
-	return get, n
+	return get, n, false
 }
 
 // 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), n int) {
+func (check *checker) arguments(x *operand, call *ast.CallExpr, sig *Signature, arg getter, n int) {
 	passSlice := false
 	if call.Ellipsis.IsValid() {
 		// last argument is of the form x...

go/types/check.go

@@ -121,15 +121,19 @@ func (check *checker) recordBuiltinType(f ast.Expr, sig *Signature) {
 	}
 }
 
-func (check *checker) recordCommaOkTypes(x ast.Expr, t1, t2 Type) {
+func (check *checker) recordCommaOkTypes(x ast.Expr, a [2]Type) {
-	assert(x != nil && isTyped(t1) && isTyped(t2) && isBoolean(t2))
+	assert(x != nil)
+	if a[0] == nil || a[1] == nil {
+		return
+	}
+	assert(isTyped(a[0]) && isTyped(a[1]) && isBoolean(a[1]))
 	if m := check.Types; m != nil {
 		for {
 			assert(m[x] != nil) // should have been recorded already
 			pos := x.Pos()
 			m[x] = NewTuple(
-				NewVar(pos, check.pkg, "", t1),
+				NewVar(pos, check.pkg, "", a[0]),
-				NewVar(pos, check.pkg, "", t2),
+				NewVar(pos, check.pkg, "", a[1]),
 			)
 			// if x is a parenthesized expression (p.X), update p.X
 			p, _ := x.(*ast.ParenExpr)

go/types/self_test.go

@@ -5,6 +5,7 @@
 package types_test
 
 import (
+	"flag"
 	"fmt"
 	"go/ast"
 	"go/parser"
@@ -17,6 +18,8 @@ import (
 	. "code.google.com/p/go.tools/go/types"
 )
 
+var benchmark = flag.Bool("b", false, "run benchmarks")
+
 func TestSelf(t *testing.T) {
 	fset := token.NewFileSet()
 	files, err := pkgFiles(fset, ".")
@@ -27,9 +30,8 @@ func TestSelf(t *testing.T) {
 	_, err = Check("go/types", fset, files)
 	if err != nil {
 		// Importing go.tools/go/exact doensn't work in the
-		// build dashboard environment at the moment. Don't
+		// build dashboard environment. Don't report an error
-		// report an error for now so that the build remains
+		// for now so that the build remains green.
-		// green.
 		// TODO(gri) fix this
 		t.Log(err) // replace w/ t.Fatal eventually
 		return
@@ -37,8 +39,8 @@ func TestSelf(t *testing.T) {
 }
 
 func TestBenchmark(t *testing.T) {
-	if testing.Short() {
+	if !*benchmark {
-		return // skip benchmark in short mode
+		return
 	}
 
 	// We're not using testing's benchmarking mechanism directly
@@ -46,13 +48,13 @@ func TestBenchmark(t *testing.T) {
 
 	for _, p := range []string{"types", "exact", "gcimporter"} {
 		path := filepath.Join("..", p)
-		benchmark(t, path, false)
+		runbench(t, path, false)
-		benchmark(t, path, true)
+		runbench(t, path, true)
 		fmt.Println()
 	}
 }
 
-func benchmark(t *testing.T, path string, ignoreFuncBodies bool) {
+func runbench(t *testing.T, path string, ignoreFuncBodies bool) {
 	fset := token.NewFileSet()
 	files, err := pkgFiles(fset, path)
 	if err != nil {

go/types/testdata/stmt0.src

@@ -21,7 +21,7 @@ func assignments0() (int, int) {
 
 	a = f0 /* ERROR "used as value" */ ()
 	a = f1()
-	a = f2 /* ERROR "used as single value" */ ()
+	a = f2 /* ERROR "assignment count mismatch" */ ()
 	a, b = f2()
 	a, b, c = f2 /* ERROR "assignment count mismatch" */ ()
 	a, b, c = f3()