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 <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>

Author: griesemer

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 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)
+		}
+		return
+	}
 
-	if len(lhs) != len(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...)
-		// don't report an error if we already reported one
-		for _, x := range rhs {
-			if x.mode == invalid {
-				return
-			}
-		}
-		check.assignError(orig_rhs, len(lhs), len(rhs))
+		check.use(orig_rhs...)
 		return
 	}
 
-	if commaOk {
-		check.assignVar(lhs[0], rhs[0])
-		check.assignVar(lhs[1], rhs[1])
-		check.recordCommaOkTypes(orig_rhs[0], rhs)
+	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
 	}
 
-	for i, lhs := range lhs {
-		check.assignVar(lhs, rhs[i])
+	// 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 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)
+		}
+		return
+	}
 
-	if len(lhs) != len(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...)
-		// don't report an error if we already reported one
-		for _, x := range rhs {
-			if x.mode == invalid {
-				return
-			}
-		}
-		check.assignError(origRHS, len(lhs), len(rhs))
+		check.use(orig_rhs...)
 		return
 	}
 
-	if commaOk {
-		check.assignVar(lhs[0], rhs[0])
-		check.assignVar(lhs[1], rhs[1])
-		check.recordCommaOkTypes(origRHS[0], rhs)
+	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
 	}
 
-	for i, lhs := range lhs {
-		check.assignVar(lhs, rhs[i])
+	// 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)