From: Robert Griesemer Date: Tue, 20 Apr 2021 23:18:59 +0000 (-0700) Subject: cmd/compile/internal/types2: better errors for invalid short var decls X-Git-Tag: go1.17beta1~507 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ece5935364;p=gostls13.git 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 Reviewed-by: Robert Findley --- diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go index ddcb5e00b3..ec9fdbba62 100644 --- 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 index 0000000000..85d4450139 --- /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 022883040b..bedcbe5fce 100644 --- 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 }