cmd/compile/internal/types2: better errors for invalid short var decls

- rewrite Checker.shortVarDecl core loop for clarity
- match compiler error messages (#43087)
- don't allow multiple identical redeclarations (#45652)

For #43087.
For #45652.

Change-Id: I8c3329a553aa104d7853fbaea8b88049bc9b3b88
Reviewed-on: https://go-review.googlesource.com/c/go/+/312170
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go
index ddcb5e00b334aa98919a120ffa3b8d2ca0d8a04b..ec9fdbba629c83bc864f8d586b94c1a04b406c79 100644 (file)
--- a/src/cmd/compile/internal/types2/assignments.go
+++ b/src/cmd/compile/internal/types2/assignments.go
@@ -330,40 +330,59 @@ func (check *Checker) shortVarDecl(pos syntax.Pos, lhs, rhs []syntax.Expr) {
        scope := check.scope
 
        // collect lhs variables
-       var newVars []*Var
-       var lhsVars = make([]*Var, len(lhs))
+       seen := make(map[string]bool, len(lhs))
+       lhsVars := make([]*Var, len(lhs))
+       newVars := make([]*Var, 0, len(lhs))
+       hasErr := false
        for i, lhs := range lhs {
-               var obj *Var
-               if ident, _ := lhs.(*syntax.Name); ident != nil {
-                       // Use the correct obj if the ident is redeclared. The
-                       // variable's scope starts after the declaration; so we
-                       // must use Scope.Lookup here and call Scope.Insert
-                       // (via check.declare) later.
-                       name := ident.Value
-                       if alt := scope.Lookup(name); alt != nil {
-                               // redeclared object must be a variable
-                               if alt, _ := alt.(*Var); alt != nil {
-                                       obj = alt
-                               } else {
-                                       check.errorf(lhs, "cannot assign to %s", lhs)
-                               }
-                               check.recordUse(ident, alt)
+               ident, _ := lhs.(*syntax.Name)
+               if ident == nil {
+                       check.useLHS(lhs)
+                       check.errorf(lhs, "non-name %s on left side of :=", lhs)
+                       hasErr = true
+                       continue
+               }
+
+               name := ident.Value
+               if name == "_" {
+                       continue
+               }
+
+               if seen[name] {
+                       check.errorf(lhs, "%s repeated on left side of :=", lhs)
+                       hasErr = true
+                       continue
+               }
+               seen[name] = true
+
+               // Use the correct obj if the ident is redeclared. The
+               // variable's scope starts after the declaration; so we
+               // must use Scope.Lookup here and call Scope.Insert
+               // (via check.declare) later.
+               if alt := scope.Lookup(name); alt != nil {
+                       check.recordUse(ident, alt)
+                       // redeclared object must be a variable
+                       if obj, _ := alt.(*Var); obj != nil {
+                               lhsVars[i] = obj
                        } else {
-                               // declare new variable, possibly a blank (_) variable
-                               obj = NewVar(ident.Pos(), check.pkg, name, nil)
-                               if name != "_" {
-                                       newVars = append(newVars, obj)
-                               }
-                               check.recordDef(ident, obj)
+                               check.errorf(lhs, "cannot assign to %s", lhs)
+                               hasErr = true
                        }
-               } else {
-                       check.useLHS(lhs)
-                       check.errorf(lhs, "cannot declare %s", lhs)
+                       continue
                }
+
+               // declare new variable
+               obj := NewVar(ident.Pos(), check.pkg, name, nil)
+               lhsVars[i] = obj
+               newVars = append(newVars, obj)
+               check.recordDef(ident, obj)
+       }
+
+       // create dummy variables where the lhs is invalid
+       for i, obj := range lhsVars {
                if obj == nil {
-                       obj = NewVar(lhs.Pos(), check.pkg, "_", nil) // dummy variable
+                       lhsVars[i] = NewVar(lhs[i].Pos(), check.pkg, "_", nil)
                }
-               lhsVars[i] = obj
        }
 
        check.initVars(lhsVars, rhs, nopos)
@@ -371,17 +390,18 @@ func (check *Checker) shortVarDecl(pos syntax.Pos, lhs, rhs []syntax.Expr) {
        // process function literals in rhs expressions before scope changes
        check.processDelayed(top)
 
-       // declare new variables
-       if len(newVars) > 0 {
-               // spec: "The scope of a constant or variable identifier declared inside
-               // a function begins at the end of the ConstSpec or VarSpec (ShortVarDecl
-               // for short variable declarations) and ends at the end of the innermost
-               // containing block."
-               scopePos := syntax.EndPos(rhs[len(rhs)-1])
-               for _, obj := range newVars {
-                       check.declare(scope, nil, obj, scopePos) // recordObject already called
-               }
-       } else {
+       if len(newVars) == 0 && !hasErr {
                check.softErrorf(pos, "no new variables on left side of :=")
+               return
+       }
+
+       // declare new variables
+       // spec: "The scope of a constant or variable identifier declared inside
+       // a function begins at the end of the ConstSpec or VarSpec (ShortVarDecl
+       // for short variable declarations) and ends at the end of the innermost
+       // containing block."
+       scopePos := syntax.EndPos(rhs[len(rhs)-1])
+       for _, obj := range newVars {
+               check.declare(scope, nil, obj, scopePos) // id = nil: recordDef already called
        }
 }

diff --git a/src/cmd/compile/internal/types2/fixedbugs/issue43087.src b/src/cmd/compile/internal/types2/fixedbugs/issue43087.src
new file mode 100644 (file)
index 0000000..85d4450
--- /dev/null
+++ b/src/cmd/compile/internal/types2/fixedbugs/issue43087.src
@@ -0,0 +1,43 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+func _() {
+       a, b, b /* ERROR b repeated on left side of := */ := 1, 2, 3
+       _ = a
+       _ = b
+}
+
+func _() {
+       a, _, _ := 1, 2, 3 // multiple _'s ok
+       _ = a
+}
+
+func _() {
+       var b int
+       a, b, b /* ERROR b repeated on left side of := */ := 1, 2, 3
+       _ = a
+       _ = b
+}
+
+func _() {
+       var a []int
+       a /* ERROR non-name .* on left side of := */ [0], b := 1, 2
+       _ = a
+       _ = b
+}
+
+func _() {
+       var a int
+       a, a /* ERROR a repeated on left side of := */ := 1, 2
+       _ = a
+}
+
+func _() {
+       var a, b int
+       a, b := /* ERROR no new variables on left side of := */ 1, 2
+       _ = a
+       _ = b
+}

diff --git a/src/cmd/compile/internal/types2/testdata/stmt0.src b/src/cmd/compile/internal/types2/testdata/stmt0.src
index 022883040bf3db2de58fd01d2377f83087eeed58..bedcbe5fce3573f8aea5c8c8b2fb73786c0a9b17 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/stmt0.src
+++ b/src/cmd/compile/internal/types2/testdata/stmt0.src
@@ -143,11 +143,11 @@ func issue6487() {
 }
 
 func issue6766a() {
-       a, a /* ERROR redeclared */ := 1, 2
+       a, a /* ERROR a repeated on left side of := */ := 1, 2
        _ = a
-       a, b, b /* ERROR redeclared */ := 1, 2, 3
+       a, b, b /* ERROR b repeated on left side of := */ := 1, 2, 3
        _ = b
-       c, c /* ERROR redeclared */, b := 1, 2, 3
+       c, c /* ERROR c repeated on left side of := */, b := 1, 2, 3
        _ = c
        a, b := /* ERROR no new variables */ 1, 2
 }