[go] types2: refactor assignVars

0 views
Skip to first unread message

Gopher Robot (Gerrit)

unread,
Mar 28, 2023, 1:34:18 PM3/28/23
to Robert Griesemer, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Robert Findley, Robert Griesemer, golang-co...@googlegroups.com

Gopher Robot submitted this change.

View Change

Approvals: Robert Griesemer: Looks good to me, but someone else must approve; Run TryBots; Automatically submit change Robert Findley: Looks good to me, approved Gopher Robot: TryBots succeeded
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 <g...@google.com>
Reviewed-by: Robert Findley <rfin...@google.com>
Run-TryBot: Robert Griesemer <g...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Auto-Submit: Robert Griesemer <g...@google.com>
---
M src/cmd/compile/internal/types2/assignments.go
M src/cmd/compile/internal/types2/expr.go
M src/go/types/assignments.go
M src/go/types/expr.go
4 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go
index 9b130b4..2d6391c 100644
--- a/src/cmd/compile/internal/types2/assignments.go
+++ b/src/cmd/compile/internal/types2/assignments.go
@@ -387,30 +387,55 @@
}

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.
diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go
index 1217d2f..fdc7bdb 100644
--- a/src/cmd/compile/internal/types2/expr.go
+++ b/src/cmd/compile/internal/types2/expr.go
@@ -1826,6 +1826,7 @@
// 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)
diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go
index 9d6a1ef..05049e0 100644
--- a/src/go/types/assignments.go
+++ b/src/go/types/assignments.go
@@ -372,31 +372,56 @@
}
}

-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) {
diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index 3a4b30d..1abf963 100644
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -1773,6 +1773,7 @@
// 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)

To view, visit change 478256. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ia43265f84e423b0ad5594612ba5a0ddce31a4a37
Gerrit-Change-Number: 478256
Gerrit-PatchSet: 15
Gerrit-Owner: Robert Griesemer <g...@golang.org>
Gerrit-Reviewer: Gopher Robot <go...@golang.org>
Gerrit-Reviewer: Robert Findley <rfin...@google.com>
Gerrit-Reviewer: Robert Griesemer <g...@google.com>
Gerrit-MessageType: merged
Reply all
Reply to author
Forward
0 new messages