[go] go/types, types2: refactor initVars

0 views
Skip to first unread message

Gopher Robot (Gerrit)

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

Gopher Robot submitted this change.

View Change



6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/go/types/assignments.go
Insertions: 5, Deletions: 2.

@@ -334,8 +334,10 @@
check.report(&err)
}

-// If returnStmt != nil, initVars is called to type-check the assignment
-// of return expressions, and returnStmt is the return statement.
+// initVars type-checks assignments of initialization expressions orig_rhs
+// to variables lhs.
+// If returnStmt is non-nil, initVars type-checks the implicit assignment
+// of result expressions orig_rhs to function result parameters lhs.
func (check *Checker) initVars(lhs []*Var, orig_rhs []ast.Expr, returnStmt ast.Stmt) {
context := "assignment"
if returnStmt != nil {
@@ -412,6 +414,7 @@
// orig_rhs[0] was already evaluated
}

+// assignVars type-checks assignments of expressions orig_rhs to variables lhs.
func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
l, r := len(lhs), len(orig_rhs)

```
```
The name of the file: src/cmd/compile/internal/types2/assignments.go
Insertions: 5, Deletions: 2.

@@ -336,8 +336,10 @@
check.report(&err)
}

-// If returnStmt != nil, initVars is called to type-check the assignment
-// of return expressions, and returnStmt is the return statement.
+// initVars type-checks assignments of initialization expressions orig_rhs
+// to variables lhs.
+// If returnStmt is non-nil, initVars type-checks the implicit assignment
+// of result expressions orig_rhs to function result parameters lhs.
func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt syntax.Stmt) {
context := "assignment"
if returnStmt != nil {
@@ -414,6 +416,7 @@
// orig_rhs[0] was already evaluated
}

+// assignVars type-checks assignments of expressions orig_rhs to variables lhs.
func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
l, r := len(lhs), len(orig_rhs)

```

Approvals: Robert Griesemer: Looks good to me, but someone else must approve; Run TryBots; Automatically submit change Gopher Robot: TryBots succeeded Robert Findley: Looks good to me, approved
go/types, types2: refactor initVars

As with changes in prior CLs, we don't suppress legitimate
"declared but not used" errors anymore simply because the
respective variables are used in incorrect assignments,
unrelated to the variables in question.
Adjust several (ancient) tests accordingly.

Change-Id: I5826393264d9d8085c64777a330d4efeb735dd2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/478716
Reviewed-by: Robert Griesemer <g...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Auto-Submit: Robert Griesemer <g...@google.com>
Reviewed-by: Robert Findley <rfin...@google.com>
Run-TryBot: Robert Griesemer <g...@google.com>
---
M src/cmd/compile/internal/types2/assignments.go
M src/go/types/assignments.go
M test/fixedbugs/bug037.go
M test/fixedbugs/bug072.go
M test/fixedbugs/bug091.go
M test/fixedbugs/bug103.go
M test/fixedbugs/bug107.go
M test/fixedbugs/bug122.go
M test/fixedbugs/bug175.go
M test/fixedbugs/issue19012.go
M test/fixedbugs/issue48558.go
11 files changed, 188 insertions(+), 99 deletions(-)

diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go
index 2d6391c..5436c46 100644
--- a/src/cmd/compile/internal/types2/assignments.go
+++ b/src/cmd/compile/internal/types2/assignments.go
@@ -305,9 +305,9 @@
return fmt.Sprintf("%d %s", x, unit)
}

-func (check *Checker) assignError(rhs []syntax.Expr, nvars, nvals int) {
- vars := measure(nvars, "variable")
- vals := measure(nvals, "value")
+func (check *Checker) assignError(rhs []syntax.Expr, l, r int) {
+ vars := measure(l, "variable")
+ vals := measure(r, "value")
rhs0 := rhs[0]

if len(rhs) == 1 {
@@ -319,73 +319,104 @@
check.errorf(rhs0, WrongAssignCount, "assignment mismatch: %s but %s", vars, vals)
}

-// If returnStmt != nil, initVars is called to type-check the assignment
-// of return expressions, and returnStmt is the return statement.
-func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt syntax.Stmt) {
- rhs, commaOk := check.exprList(orig_rhs, len(lhs) == 2 && returnStmt == nil)
-
- if len(lhs) != len(rhs) {
- // invalidate lhs
- for _, obj := range lhs {
- obj.used = true // avoid declared and not used errors
- if obj.typ == nil {
- obj.typ = Typ[Invalid]
- }
- }
- // don't report an error if we already reported one
- for _, x := range rhs {
- if x.mode == invalid {
- return
- }
- }
- if returnStmt != nil {
- var at poser = returnStmt
- qualifier := "not enough"
- if len(rhs) > len(lhs) {
- at = rhs[len(lhs)].expr // report at first extra value
- qualifier = "too many"
- } else if len(rhs) > 0 {
- at = rhs[len(rhs)-1].expr // report at last value
- }
- var err error_
- err.code = WrongResultCount
- err.errorf(at, "%s return values", qualifier)
- err.errorf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false))
- err.errorf(nopos, "want %s", check.typesSummary(varTypes(lhs), false))
- check.report(&err)
- return
- }
- check.assignError(orig_rhs, len(lhs), len(rhs))
- return
+func (check *Checker) returnError(at poser, lhs []*Var, rhs []*operand) {
+ l, r := len(lhs), len(rhs)
+ qualifier := "not enough"
+ if r > l {
+ at = rhs[l] // report at first extra value
+ qualifier = "too many"
+ } else if r > 0 {
+ at = rhs[r-1] // report at last value
}
+ var err error_
+ err.code = WrongResultCount
+ err.errorf(at, "%s return values", qualifier)
+ err.errorf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false))
+ err.errorf(nopos, "want %s", check.typesSummary(varTypes(lhs), false))
+ check.report(&err)
+}

+// initVars type-checks assignments of initialization expressions orig_rhs
+// to variables lhs.
+// If returnStmt is non-nil, initVars type-checks the implicit assignment
+// of result expressions orig_rhs to function result parameters lhs.
+func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt syntax.Stmt) {
context := "assignment"
if returnStmt != nil {
context = "return statement"
}

- if commaOk {
- check.initVar(lhs[0], rhs[0], context)
- check.initVar(lhs[1], rhs[1], context)
- check.recordCommaOkTypes(orig_rhs[0], rhs)
+ 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 {
+ var x operand
+ for i, lhs := range lhs {
+ check.expr(&x, orig_rhs[i])
+ check.initVar(lhs, &x, context)
+ }
return
}

- ok := true
- for i, lhs := range lhs {
- if check.initVar(lhs, rhs[i], context) == nil {
- ok = false
+ // 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, false)
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
}
+ // ensure that LHS variables have a type
+ for _, v := range lhs {
+ if v.typ == nil {
+ v.typ = Typ[Invalid]
+ }
+ }
+ check.use(orig_rhs...)
+ return
}

- // avoid follow-on "declared and not used" errors if any initialization failed
- if !ok {
- for _, lhs := range lhs {
- lhs.used = true
+ rhs, commaOk := check.multiExpr(orig_rhs[0], l == 2 && returnStmt == nil)
+ r = len(rhs)
+ if l == r {
+ for i, lhs := range lhs {
+ check.initVar(lhs, rhs[i], context)
+ }
+ 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 {
+ if returnStmt != nil {
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
}
}
+ // ensure that LHS variables have a type
+ for _, v := range lhs {
+ if v.typ == nil {
+ v.typ = Typ[Invalid]
+ }
+ }
+ // orig_rhs[0] was already evaluated
}

+// assignVars type-checks assignments of expressions orig_rhs to variables lhs.
func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
l, r := len(lhs), len(orig_rhs)

diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go
index 05049e0..84b45f1 100644
--- a/src/go/types/assignments.go
+++ b/src/go/types/assignments.go
@@ -303,9 +303,9 @@
return fmt.Sprintf("%d %s", x, unit)
}

-func (check *Checker) assignError(rhs []ast.Expr, nvars, nvals int) {
- vars := measure(nvars, "variable")
- vals := measure(nvals, "value")
+func (check *Checker) assignError(rhs []ast.Expr, l, r int) {
+ vars := measure(l, "variable")
+ vals := measure(r, "value")
rhs0 := rhs[0]

if len(rhs) == 1 {
@@ -317,61 +317,104 @@
check.errorf(rhs0, WrongAssignCount, "assignment mismatch: %s but %s", vars, vals)
}

-// If returnStmt != nil, initVars is called to type-check the assignment
-// of return expressions, and returnStmt is the return statement.
-func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnStmt ast.Stmt) {
- rhs, commaOk := check.exprList(origRHS, len(lhs) == 2 && returnStmt == nil)
-
- if len(lhs) != len(rhs) {
- // invalidate lhs
- for _, obj := range lhs {
- obj.used = true // avoid declared and not used errors
- if obj.typ == nil {
- obj.typ = Typ[Invalid]
- }
- }
- // don't report an error if we already reported one
- for _, x := range rhs {
- if x.mode == invalid {
- return
- }
- }
- if returnStmt != nil {
- var at positioner = returnStmt
- qualifier := "not enough"
- if len(rhs) > len(lhs) {
- at = rhs[len(lhs)].expr // report at first extra value
- qualifier = "too many"
- } else if len(rhs) > 0 {
- at = rhs[len(rhs)-1].expr // report at last value
- }
- err := newErrorf(at, WrongResultCount, "%s return values", qualifier)
- err.errorf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false))
- err.errorf(nopos, "want %s", check.typesSummary(varTypes(lhs), false))
- check.report(err)
- return
- }
- check.assignError(origRHS, len(lhs), len(rhs))
- return
+func (check *Checker) returnError(at positioner, lhs []*Var, rhs []*operand) {
+ l, r := len(lhs), len(rhs)
+ qualifier := "not enough"
+ if r > l {
+ at = rhs[l] // report at first extra value
+ qualifier = "too many"
+ } else if r > 0 {
+ at = rhs[r-1] // report at last value
}
+ var err error_
+ err.code = WrongResultCount
+ err.errorf(at.Pos(), "%s return values", qualifier)
+ err.errorf(nopos, "have %s", check.typesSummary(operandTypes(rhs), false))
+ err.errorf(nopos, "want %s", check.typesSummary(varTypes(lhs), false))
+ check.report(&err)
+}

+// initVars type-checks assignments of initialization expressions orig_rhs
+// to variables lhs.
+// If returnStmt is non-nil, initVars type-checks the implicit assignment
+// of result expressions orig_rhs to function result parameters lhs.
+func (check *Checker) initVars(lhs []*Var, orig_rhs []ast.Expr, returnStmt ast.Stmt) {
context := "assignment"
if returnStmt != nil {
context = "return statement"
}

- if commaOk {
- check.initVar(lhs[0], rhs[0], context)
- check.initVar(lhs[1], rhs[1], context)
- check.recordCommaOkTypes(origRHS[0], rhs)
+ 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 {
+ var x operand
+ for i, lhs := range lhs {
+ check.expr(&x, orig_rhs[i])
+ check.initVar(lhs, &x, context)
+ }
return
}

- for i, lhs := range lhs {
- check.initVar(lhs, rhs[i], context)
+ // 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, false)
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
+ }
+ // ensure that LHS variables have a type
+ for _, v := range lhs {
+ if v.typ == nil {
+ v.typ = Typ[Invalid]
+ }
+ }
+ check.use(orig_rhs...)
+ return
}
+
+ rhs, commaOk := check.multiExpr(orig_rhs[0], l == 2 && returnStmt == nil)
+ r = len(rhs)
+ if l == r {
+ for i, lhs := range lhs {
+ check.initVar(lhs, rhs[i], context)
+ }
+ 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 {
+ if returnStmt != nil {
+ check.returnError(returnStmt, lhs, rhs)
+ } else {
+ check.assignError(orig_rhs, l, r)
+ }
+ }
+ // ensure that LHS variables have a type
+ for _, v := range lhs {
+ if v.typ == nil {
+ v.typ = Typ[Invalid]
+ }
+ }
+ // orig_rhs[0] was already evaluated
}

+// assignVars type-checks assignments of expressions orig_rhs to variables lhs.
func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
l, r := len(lhs), len(orig_rhs)

diff --git a/test/fixedbugs/bug037.go b/test/fixedbugs/bug037.go
index f17fb3f..ed95cac 100644
--- a/test/fixedbugs/bug037.go
+++ b/test/fixedbugs/bug037.go
@@ -8,4 +8,5 @@

func main() {
s := vlong(0); // ERROR "undef"
+ _ = s
}
diff --git a/test/fixedbugs/bug072.go b/test/fixedbugs/bug072.go
index 05ad93d..1c0c4ee 100644
--- a/test/fixedbugs/bug072.go
+++ b/test/fixedbugs/bug072.go
@@ -8,4 +8,5 @@

func main() {
s := string(bug); // ERROR "undef"
+ _ = s
}
diff --git a/test/fixedbugs/bug091.go b/test/fixedbugs/bug091.go
index dbb1287..0e239e0 100644
--- a/test/fixedbugs/bug091.go
+++ b/test/fixedbugs/bug091.go
@@ -18,6 +18,7 @@

func f3() {
i := c // ERROR "undef"
+ _ = i
}

func main() {
diff --git a/test/fixedbugs/bug103.go b/test/fixedbugs/bug103.go
index 1cb710e..743a3c4 100644
--- a/test/fixedbugs/bug103.go
+++ b/test/fixedbugs/bug103.go
@@ -10,5 +10,6 @@

func main() {
x := f(); // ERROR "mismatch|as value|no type"
+ _ = x
}

diff --git a/test/fixedbugs/bug107.go b/test/fixedbugs/bug107.go
index dcd8e9d..e4b9eb1 100644
--- a/test/fixedbugs/bug107.go
+++ b/test/fixedbugs/bug107.go
@@ -11,5 +11,6 @@
// In the next line "os" should refer to the result variable, not
// to the package.
v := os.Open("", 0, 0); // ERROR "undefined"
+ _ = v
return 0
}
diff --git a/test/fixedbugs/bug122.go b/test/fixedbugs/bug122.go
index 5640cf2..0d9dcd1 100644
--- a/test/fixedbugs/bug122.go
+++ b/test/fixedbugs/bug122.go
@@ -9,4 +9,5 @@
func main() {
// should allow at most 2 sizes
a := make([]int, 10, 20, 30, 40); // ERROR "too many|expects 2 or 3 arguments; found 5"
+ _ = a
}
diff --git a/test/fixedbugs/bug175.go b/test/fixedbugs/bug175.go
index caf3168..f19025a 100644
--- a/test/fixedbugs/bug175.go
+++ b/test/fixedbugs/bug175.go
@@ -10,4 +10,5 @@

func main() {
x, y := f(), 2 // ERROR "multi|2-valued"
+ _, _ = x, y
}
diff --git a/test/fixedbugs/issue19012.go b/test/fixedbugs/issue19012.go
index 77b2236..c911a9a 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"
+ 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"
}

func main() {
diff --git a/test/fixedbugs/issue48558.go b/test/fixedbugs/issue48558.go
index 9ab56d9..590fd9b 100644
--- a/test/fixedbugs/issue48558.go
+++ b/test/fixedbugs/issue48558.go
@@ -41,6 +41,10 @@
a1 := f3() // ERROR "assignment mismatch: 1 variable but f3 returns 3 values"
a2, b2 := f1() // ERROR "assignment mismatch: 2 variables but f1 returns 1 value"
a3, b3, c3 := f2() // ERROR "assignment mismatch: 3 variables but f2 returns 2 values"
+
+ _ = a1
+ _, _ = a2, b2
+ _, _, _ = a3, b3, c3
}

type T struct{}
@@ -66,6 +70,10 @@
a1 := x.f3() // ERROR "assignment mismatch: 1 variable but .\.f3 returns 3 values"
a2, b2 := x.f1() // ERROR "assignment mismatch: 2 variables but .\.f1 returns 1 value"
a3, b3, c3 := x.f2() // ERROR "assignment mismatch: 3 variables but .\.f2 returns 2 values"
+
+ _ = a1
+ _, _ = a2, b2
+ _, _, _ = a3, b3, c3
}

// some one-off cases

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

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5826393264d9d8085c64777a330d4efeb735dd2d
Gerrit-Change-Number: 478716
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