[go] go/types, types2: don't report assignment mismatch errors if there are other errors

0 views
Skip to first unread message

Gopher Robot (Gerrit)

unread,
Mar 28, 2023, 6:22:14 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: don't report assignment mismatch errors if there are other errors

Change the Checker.use/useLHS functions to report if all "used"
expressions evaluated without error. Use that information to
control whether to report an assignment mismatch error or not.
This will reduce the number of errors reported per assignment,
where the assignment mismatch is only one of the errors.

Change-Id: Ia0fc3203253b002e4e1d5759d8d5644999af6884
Reviewed-on: https://go-review.googlesource.com/c/go/+/478756
Reviewed-by: Robert Findley <rfin...@google.com>
Reviewed-by: Robert Griesemer <g...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Robert Griesemer <g...@google.com>
Auto-Submit: Robert Griesemer <g...@google.com>
---
M src/cmd/compile/internal/types2/assignments.go
M src/cmd/compile/internal/types2/call.go
M src/go/types/assignments.go
M src/go/types/call.go
M test/fixedbugs/issue19012.go
5 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go
index adef1e8..3ca6beb 100644
--- a/src/cmd/compile/internal/types2/assignments.go
+++ b/src/cmd/compile/internal/types2/assignments.go
@@ -360,11 +360,14 @@
// 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 {
- if returnStmt != nil {
- rhs := check.exprList(orig_rhs)
- check.returnError(returnStmt, lhs, rhs)
- } else {
- check.assignError(orig_rhs, l, r)
+ // Only report a mismatch error if there are no other errors on the rhs.
+ if check.use(orig_rhs...) {
+ if returnStmt != nil {
+ rhs := check.exprList(orig_rhs)
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
+ }
}
// ensure that LHS variables have a type
for _, v := range lhs {
@@ -372,7 +375,6 @@
v.typ = Typ[Invalid]
}
}
- check.use(orig_rhs...)
return
}

@@ -389,8 +391,7 @@
}

// In all other cases we have an assignment mismatch.
- // Only report a mismatch error if there was no error
- // on the rhs.
+ // Only report a mismatch error if there are no other errors on the rhs.
if rhs[0].mode != invalid {
if returnStmt != nil {
check.returnError(returnStmt, lhs, rhs)
@@ -432,9 +433,12 @@
// 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...)
+ // Only report a mismatch error if there are no other errors on the lhs or rhs.
+ okLHS := check.useLHS(lhs...)
+ okRHS := check.use(orig_rhs...)
+ if okLHS && okRHS {
+ check.assignError(orig_rhs, l, r)
+ }
return
}

@@ -451,8 +455,7 @@
}

// In all other cases we have an assignment mismatch.
- // Only report a mismatch error if there was no error
- // on the rhs.
+ // Only report a mismatch error if there are no other errors on the rhs.
if rhs[0].mode != invalid {
check.assignError(orig_rhs, l, r)
}
diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go
index 1400aba..72608de 100644
--- a/src/cmd/compile/internal/types2/call.go
+++ b/src/cmd/compile/internal/types2/call.go
@@ -699,23 +699,27 @@
// Useful to make sure expressions are evaluated
// (and variables are "used") in the presence of
// other errors. Arguments may be nil.
-func (check *Checker) use(args ...syntax.Expr) {
- for _, e := range args {
- check.use1(e, false)
- }
-}
+// Reports if all arguments evaluated without error.
+func (check *Checker) use(args ...syntax.Expr) bool { return check.useN(args, false) }

// useLHS is like use, but doesn't "use" top-level identifiers.
// It should be called instead of use if the arguments are
// expressions on the lhs of an assignment.
-func (check *Checker) useLHS(args ...syntax.Expr) {
+func (check *Checker) useLHS(args ...syntax.Expr) bool { return check.useN(args, true) }
+
+func (check *Checker) useN(args []syntax.Expr, lhs bool) bool {
+ ok := true
for _, e := range args {
- check.use1(e, true)
+ if !check.use1(e, lhs) {
+ ok = false
+ }
}
+ return ok
}

-func (check *Checker) use1(e syntax.Expr, lhs bool) {
+func (check *Checker) use1(e syntax.Expr, lhs bool) bool {
var x operand
+ x.mode = value // anything but invalid
switch n := unparen(e).(type) {
case nil:
// nothing to do
@@ -745,10 +749,9 @@
v.used = v_used // restore v.used
}
case *syntax.ListExpr:
- for _, e := range n.ElemList {
- check.use1(e, lhs)
- }
+ return check.useN(n.ElemList, lhs)
default:
check.rawExpr(&x, e, nil, true)
}
+ return x.mode != invalid
}
diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go
index 26de0a0..a73e451 100644
--- a/src/go/types/assignments.go
+++ b/src/go/types/assignments.go
@@ -358,11 +358,14 @@
// 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 {
- if returnStmt != nil {
- rhs := check.exprList(orig_rhs)
- check.returnError(returnStmt, lhs, rhs)
- } else {
- check.assignError(orig_rhs, l, r)
+ // Only report a mismatch error if there are no other errors on the rhs.
+ if check.use(orig_rhs...) {
+ if returnStmt != nil {
+ rhs := check.exprList(orig_rhs)
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
+ }
}
// ensure that LHS variables have a type
for _, v := range lhs {
@@ -370,7 +373,6 @@
v.typ = Typ[Invalid]
}
}
- check.use(orig_rhs...)
return
}

@@ -387,8 +389,7 @@
}

// In all other cases we have an assignment mismatch.
- // Only report a mismatch error if there was no error
- // on the rhs.
+ // Only report a mismatch error if there are no other errors on the rhs.
if rhs[0].mode != invalid {
if returnStmt != nil {
check.returnError(returnStmt, lhs, rhs)
@@ -430,9 +431,12 @@
// 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...)
+ // Only report a mismatch error if there are no other errors on the lhs or rhs.
+ okLHS := check.useLHS(lhs...)
+ okRHS := check.use(orig_rhs...)
+ if okLHS && okRHS {
+ check.assignError(orig_rhs, l, r)
+ }
return
}

@@ -449,8 +453,7 @@
}

// In all other cases we have an assignment mismatch.
- // Only report a mismatch error if there was no error
- // on the rhs.
+ // Only report a mismatch error if there are no other errors on the rhs.
if rhs[0].mode != invalid {
check.assignError(orig_rhs, l, r)
}
diff --git a/src/go/types/call.go b/src/go/types/call.go
index f8aa261..e5968c7 100644
--- a/src/go/types/call.go
+++ b/src/go/types/call.go
@@ -746,23 +746,27 @@
// Useful to make sure expressions are evaluated
// (and variables are "used") in the presence of
// other errors. Arguments may be nil.
-func (check *Checker) use(args ...ast.Expr) {
- for _, e := range args {
- check.use1(e, false)
- }
-}
+// Reports if all arguments evaluated without error.
+func (check *Checker) use(args ...ast.Expr) bool { return check.useN(args, false) }

// useLHS is like use, but doesn't "use" top-level identifiers.
// It should be called instead of use if the arguments are
// expressions on the lhs of an assignment.
-func (check *Checker) useLHS(args ...ast.Expr) {
+func (check *Checker) useLHS(args ...ast.Expr) bool { return check.useN(args, true) }
+
+func (check *Checker) useN(args []ast.Expr, lhs bool) bool {
+ ok := true
for _, e := range args {
- check.use1(e, true)
+ if !check.use1(e, lhs) {
+ ok = false
+ }
}
+ return ok
}

-func (check *Checker) use1(e ast.Expr, lhs bool) {
+func (check *Checker) use1(e ast.Expr, lhs bool) bool {
var x operand
+ x.mode = value // anything but invalid
switch n := unparen(e).(type) {
case nil:
// nothing to do
@@ -794,4 +798,5 @@
default:
check.rawExpr(&x, e, nil, true)
}
+ return x.mode != invalid
}
diff --git a/test/fixedbugs/issue19012.go b/test/fixedbugs/issue19012.go
index c911a9a..77b2236 100644
--- a/test/fixedbugs/issue19012.go
+++ b/test/fixedbugs/issue19012.go
@@ -15,7 +15,7 @@
if true {
return "a" > 10 // ERROR "^too many arguments to return$|return with value in function with no return|no result values expected|mismatched types"
}
- return "gopher" == true, 10 // ERROR "^too many arguments to return$|return with value in function with no return|no result values expected|mismatched types" "too many return values"
+ return "gopher" == true, 10 // ERROR "^too many arguments to return$|return with value in function with no return|no result values expected|mismatched types"
}

func main() {

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

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ia0fc3203253b002e4e1d5759d8d5644999af6884
Gerrit-Change-Number: 478756
Gerrit-PatchSet: 9
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