go/types, types2: refactor assignVars

Rather than using exprList and handle all cases together, split
apart the cases of n:n assignments and the cases of n:1 assignments.
For the former, the lhs types may (in a future CL) be used to infer
types on the rhs. This is a preparatory step.

Because the two cases are handled separately, the code is longer
(but also more explicit).

Some test cases were adjusted to avoifd (legitimate, but previously
supressed) "declared but not used" errors.

Change-Id: Ia43265f84e423b0ad5594612ba5a0ddce31a4a37
Reviewed-on: https://go-review.googlesource.com/c/go/+/478256
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>

src/cmd/compile/internal/types2/assignments.go

@@ -387,30 +387,55 @@ func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt sy
 }
 
 func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
-	rhs, commaOk := check.exprList(orig_rhs, len(lhs) == 2)
+	l, r := len(lhs), len(orig_rhs)
 
-	if len(lhs) != len(rhs) {
-		check.useLHS(lhs...)
-		// don't report an error if we already reported one
-		for _, x := range rhs {
-			if x.mode == invalid {
-				return
-			}
+	// If l == 1 and the rhs is a single call, for a better
+	// error message don't handle it as n:n mapping below.
+	isCall := false
+	if r == 1 {
+		_, isCall = unparen(orig_rhs[0]).(*syntax.CallExpr)
+	}
+
+	// If we have a n:n mapping from lhs variable to rhs expression,
+	// each value can be assigned to its corresponding variable.
+	if l == r && !isCall {
+		for i, lhs := range lhs {
+			var x operand
+			check.expr(&x, orig_rhs[i])
+			check.assignVar(lhs, &x)
 		}
-		check.assignError(orig_rhs, len(lhs), len(rhs))
 		return
 	}
 
-	if commaOk {
-		check.assignVar(lhs[0], rhs[0])
-		check.assignVar(lhs[1], rhs[1])
-		check.recordCommaOkTypes(orig_rhs[0], rhs)
+	// If we don't have an n:n mapping, the rhs must be a single expression
+	// resulting in 2 or more values; otherwise we have an assignment mismatch.
+	if r != 1 {
+		check.assignError(orig_rhs, l, r)
+		check.useLHS(lhs...)
+		check.use(orig_rhs...)
 		return
 	}
 
-	for i, lhs := range lhs {
-		check.assignVar(lhs, rhs[i])
+	rhs, commaOk := check.multiExpr(orig_rhs[0], l == 2)
+	r = len(rhs)
+	if l == r {
+		for i, lhs := range lhs {
+			check.assignVar(lhs, rhs[i])
+		}
+		if commaOk {
+			check.recordCommaOkTypes(orig_rhs[0], rhs)
+		}
+		return
 	}
+
+	// In all other cases we have an assignment mismatch.
+	// Only report a mismatch error if there was no error
+	// on the rhs.
+	if rhs[0].mode != invalid {
+		check.assignError(orig_rhs, l, r)
+	}
+	check.useLHS(lhs...)
+	// orig_rhs[0] was already evaluated
 }
 
 // unpackExpr unpacks a *syntax.ListExpr into a list of syntax.Expr.

src/cmd/compile/internal/types2/expr.go

@@ -1826,6 +1826,7 @@ func (check *Checker) expr(x *operand, e syntax.Expr) {
 // If allowCommaOk is set and e is a map index, comma-ok, or comma-err
 // expression, the result is a two-element list containing the value
 // of e, and an untyped bool value or an error value, respectively.
+// If an error occurred, list[0] is not valid.
 func (check *Checker) multiExpr(e syntax.Expr, allowCommaOk bool) (list []*operand, commaOk bool) {
 	var x operand
 	check.rawExpr(&x, e, nil, false)

src/go/types/assignments.go

@@ -372,31 +372,56 @@ func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnStmt ast.St
 	}
 }
 
-func (check *Checker) assignVars(lhs, origRHS []ast.Expr) {
-	rhs, commaOk := check.exprList(origRHS, len(lhs) == 2)
+func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
+	l, r := len(lhs), len(orig_rhs)
 
-	if len(lhs) != len(rhs) {
-		check.useLHS(lhs...)
-		// don't report an error if we already reported one
-		for _, x := range rhs {
-			if x.mode == invalid {
-				return
-			}
+	// If l == 1 and the rhs is a single call, for a better
+	// error message don't handle it as n:n mapping below.
+	isCall := false
+	if r == 1 {
+		_, isCall = unparen(orig_rhs[0]).(*ast.CallExpr)
+	}
+
+	// If we have a n:n mapping from lhs variable to rhs expression,
+	// each value can be assigned to its corresponding variable.
+	if l == r && !isCall {
+		for i, lhs := range lhs {
+			var x operand
+			check.expr(&x, orig_rhs[i])
+			check.assignVar(lhs, &x)
 		}
-		check.assignError(origRHS, len(lhs), len(rhs))
 		return
 	}
 
-	if commaOk {
-		check.assignVar(lhs[0], rhs[0])
-		check.assignVar(lhs[1], rhs[1])
-		check.recordCommaOkTypes(origRHS[0], rhs)
+	// If we don't have an n:n mapping, the rhs must be a single expression
+	// resulting in 2 or more values; otherwise we have an assignment mismatch.
+	if r != 1 {
+		check.assignError(orig_rhs, l, r)
+		check.useLHS(lhs...)
+		check.use(orig_rhs...)
 		return
 	}
 
-	for i, lhs := range lhs {
-		check.assignVar(lhs, rhs[i])
+	rhs, commaOk := check.multiExpr(orig_rhs[0], l == 2)
+	r = len(rhs)
+	if l == r {
+		for i, lhs := range lhs {
+			check.assignVar(lhs, rhs[i])
+		}
+		if commaOk {
+			check.recordCommaOkTypes(orig_rhs[0], rhs)
+		}
+		return
 	}
+
+	// In all other cases we have an assignment mismatch.
+	// Only report a mismatch error if there was no error
+	// on the rhs.
+	if rhs[0].mode != invalid {
+		check.assignError(orig_rhs, l, r)
+	}
+	check.useLHS(lhs...)
+	// orig_rhs[0] was already evaluated
 }
 
 func (check *Checker) shortVarDecl(pos positioner, lhs, rhs []ast.Expr) {

src/go/types/expr.go

@@ -1773,6 +1773,7 @@ func (check *Checker) expr(x *operand, e ast.Expr) {
 // If allowCommaOk is set and e is a map index, comma-ok, or comma-err
 // expression, the result is a two-element list containing the value
 // of e, and an untyped bool value or an error value, respectively.
+// If an error occurred, list[0] is not valid.
 func (check *Checker) multiExpr(e ast.Expr, allowCommaOk bool) (list []*operand, commaOk bool) {
 	var x operand
 	check.rawExpr(&x, e, nil, false)