]> Cypherpunks.ru repositories - gostls13.git/commitdiff
go/types, types2: more systematic use of Checker.use und useLHS
authorRobert Griesemer <gri@golang.org>
Wed, 22 Mar 2023 18:59:39 +0000 (11:59 -0700)
committerGopher Robot <gobot@golang.org>
Tue, 28 Mar 2023 14:28:33 +0000 (14:28 +0000)
This CL re-introduces useLHS because we don't want to suppress
correct "declared but not used" errors for variables that only
appear on the LHS of an assignment (using Checker.use would mark
them as used).

This CL also adjusts a couple of places where types2 differed
from go/types (and suppressed valid "declared and not used"
errors). Now those errors are surfaced. Adjusted a handful of
tests accordingly.

Change-Id: Ia555139a05049887aeeec9e5221b1f41432c1a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/478635
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>

src/cmd/compile/internal/types2/assignments.go
src/cmd/compile/internal/types2/call.go
src/go/types/assignments.go
src/go/types/call.go
test/fixedbugs/bug062.go
test/fixedbugs/bug131.go
test/fixedbugs/bug289.go
test/fixedbugs/issue48471.go
test/fixedbugs/issue9083.go
test/interface/pointer.go

index c774658a23152951a2f96432ce11aa2be76f168c..afbb1186b5a6df7ae95ca3a1aa3d4b06214e9942 100644 (file)
@@ -134,9 +134,6 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) Type {
                if lhs.typ == nil {
                        lhs.typ = Typ[Invalid]
                }
-               // Note: This was reverted in go/types (https://golang.org/cl/292751).
-               // TODO(gri): decide what to do (also affects test/run.go exclusion list)
-               lhs.used = true // avoid follow-on "declared and not used" errors
                return nil
        }
 
@@ -157,7 +154,6 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) Type {
 
        check.assignment(x, lhs.typ, context)
        if x.mode == invalid {
-               lhs.used = true // avoid follow-on "declared and not used" errors
                return nil
        }
 
@@ -233,7 +229,7 @@ func (check *Checker) lhsVar(lhs syntax.Expr) Type {
 // If the assignment is invalid, the result is nil.
 func (check *Checker) assignVar(lhs syntax.Expr, x *operand) Type {
        if x.mode == invalid || x.typ == Typ[Invalid] {
-               check.use(lhs)
+               check.useLHS(lhs)
                return nil
        }
 
@@ -398,7 +394,7 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
        rhs, commaOk := check.exprList(orig_rhs, len(lhs) == 2)
 
        if len(lhs) != len(rhs) {
-               check.use(lhs...)
+               check.useLHS(lhs...)
                // don't report an error if we already reported one
                for _, x := range rhs {
                        if x.mode == invalid {
@@ -416,26 +412,8 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
                return
        }
 
-       ok := true
        for i, lhs := range lhs {
-               if check.assignVar(lhs, rhs[i]) == nil {
-                       ok = false
-               }
-       }
-
-       // avoid follow-on "declared and not used" errors if any assignment failed
-       if !ok {
-               // don't call check.use to avoid re-evaluation of the lhs expressions
-               for _, lhs := range lhs {
-                       if name, _ := unparen(lhs).(*syntax.Name); name != nil {
-                               if obj := check.lookup(name.Value); obj != nil {
-                                       // see comment in assignVar
-                                       if v, _ := obj.(*Var); v != nil && v.pkg == check.pkg {
-                                               v.used = true
-                                       }
-                               }
-                       }
-               }
+               check.assignVar(lhs, rhs[i])
        }
 }
 
@@ -466,7 +444,7 @@ func (check *Checker) shortVarDecl(pos syntax.Pos, lhs, rhs []syntax.Expr) {
        for i, lhs := range lhs {
                ident, _ := lhs.(*syntax.Name)
                if ident == nil {
-                       check.use(lhs)
+                       check.useLHS(lhs)
                        check.errorf(lhs, BadDecl, "non-name %s on left side of :=", lhs)
                        hasErr = true
                        continue
index 4a4c77decf90855f6570c58fec053e63e090f237..517befe5dddae460777e7b93f9b2ce6d5cc19f0d 100644 (file)
@@ -697,26 +697,58 @@ Error:
 
 // use type-checks each argument.
 // Useful to make sure expressions are evaluated
-// (and variables are "used") in the presence of other errors.
-// The arguments may be nil.
-// TODO(gri) make this accept a []syntax.Expr and use an unpack function when we have a ListExpr?
-func (check *Checker) use(arg ...syntax.Expr) {
+// (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)
+       }
+}
+
+// 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) {
+       for _, e := range args {
+               check.use1(e, true)
+       }
+}
+
+func (check *Checker) use1(e syntax.Expr, lhs bool) {
        var x operand
-       for _, e := range arg {
-               switch n := e.(type) {
-               case nil:
-                       // some AST fields may be nil (e.g., elements of syntax.SliceExpr.Index)
-                       // TODO(gri) can those fields really make it here?
-                       continue
-               case *syntax.Name:
-                       // don't report an error evaluating blank
-                       if n.Value == "_" {
-                               continue
+       switch n := unparen(e).(type) {
+       case nil:
+               // nothing to do
+       case *syntax.Name:
+               // don't report an error evaluating blank
+               if n.Value == "_" {
+                       break
+               }
+               // If the lhs is an identifier denoting a variable v, this assignment
+               // is not a 'use' of v. Remember current value of v.used and restore
+               // after evaluating the lhs via check.rawExpr.
+               var v *Var
+               var v_used bool
+               if lhs {
+                       if _, obj := check.scope.LookupParent(n.Value, nopos); obj != nil {
+                               // It's ok to mark non-local variables, but ignore variables
+                               // from other packages to avoid potential race conditions with
+                               // dot-imported variables.
+                               if w, _ := obj.(*Var); w != nil && w.pkg == check.pkg {
+                                       v = w
+                                       v_used = v.used
+                               }
                        }
-               case *syntax.ListExpr:
-                       check.use(n.ElemList...)
-                       continue
                }
-               check.rawExpr(&x, e, nil, false)
+               check.rawExpr(&x, n, nil, true)
+               if v != nil {
+                       v.used = v_used // restore v.used
+               }
+       case *syntax.ListExpr:
+               for _, e := range n.ElemList {
+                       check.use1(e, lhs)
+               }
+       default:
+               check.rawExpr(&x, e, nil, true)
        }
 }
index 373b8ec23173a5711486da4b342108aff7bb08c3..e1b22d16ad42468093d77f14a942de204d6a2068 100644 (file)
@@ -227,7 +227,7 @@ func (check *Checker) lhsVar(lhs ast.Expr) Type {
 // If the assignment is invalid, the result is nil.
 func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
        if x.mode == invalid || x.typ == Typ[Invalid] {
-               check.use(lhs)
+               check.useLHS(lhs)
                return nil
        }
 
@@ -380,7 +380,7 @@ func (check *Checker) assignVars(lhs, origRHS []ast.Expr) {
        rhs, commaOk := check.exprList(origRHS, len(lhs) == 2)
 
        if len(lhs) != len(rhs) {
-               check.use(lhs...)
+               check.useLHS(lhs...)
                // don't report an error if we already reported one
                for _, x := range rhs {
                        if x.mode == invalid {
@@ -415,7 +415,7 @@ func (check *Checker) shortVarDecl(pos positioner, lhs, rhs []ast.Expr) {
        for i, lhs := range lhs {
                ident, _ := lhs.(*ast.Ident)
                if ident == nil {
-                       check.use(lhs)
+                       check.useLHS(lhs)
                        // TODO(rFindley) this is redundant with a parser error. Consider omitting?
                        check.errorf(lhs, BadDecl, "non-name %s on left side of :=", lhs)
                        hasErr = true
index dce05eb4d4db926b571cd7e9fd2ea449a2078d67..47734e872b25bb2ce4df530825769cc55a9f01c3 100644 (file)
@@ -744,22 +744,54 @@ Error:
 
 // use type-checks each argument.
 // Useful to make sure expressions are evaluated
-// (and variables are "used") in the presence of other errors.
-// The arguments may be nil.
-func (check *Checker) use(arg ...ast.Expr) {
+// (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)
+       }
+}
+
+// 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) {
+       for _, e := range args {
+               check.use1(e, true)
+       }
+}
+
+func (check *Checker) use1(e ast.Expr, lhs bool) {
        var x operand
-       for _, e := range arg {
-               switch n := e.(type) {
-               case nil:
-                       // some AST fields may be nil (e.g., the ast.SliceExpr.High field)
-                       // TODO(gri) can those fields really make it here?
-                       continue
-               case *ast.Ident:
-                       // don't report an error evaluating blank
-                       if n.Name == "_" {
-                               continue
+       switch n := unparen(e).(type) {
+       case nil:
+               // nothing to do
+       case *ast.Ident:
+               // don't report an error evaluating blank
+               if n.Name == "_" {
+                       break
+               }
+               // If the lhs is an identifier denoting a variable v, this assignment
+               // is not a 'use' of v. Remember current value of v.used and restore
+               // after evaluating the lhs via check.rawExpr.
+               var v *Var
+               var v_used bool
+               if lhs {
+                       if _, obj := check.scope.LookupParent(n.Name, nopos); obj != nil {
+                               // It's ok to mark non-local variables, but ignore variables
+                               // from other packages to avoid potential race conditions with
+                               // dot-imported variables.
+                               if w, _ := obj.(*Var); w != nil && w.pkg == check.pkg {
+                                       v = w
+                                       v_used = v.used
+                               }
                        }
                }
-               check.rawExpr(&x, e, nil, false)
+               check.rawExpr(&x, n, nil, true)
+               if v != nil {
+                       v.used = v_used // restore v.used
+               }
+       default:
+               check.rawExpr(&x, e, nil, true)
        }
 }
index 1008f1af9ceca8b1d28c58537eef5519636e9d92..ef9ed5cf289c886c2decd7d676d6470391335f25 100644 (file)
@@ -7,5 +7,5 @@
 package main
 
 func main() {
-       var s string = nil // ERROR "illegal|invalid|incompatible|cannot"
+       var _ string = nil // ERROR "illegal|invalid|incompatible|cannot"
 }
index de606da1679d979ca4001f395b2aea552361ca91..511928ffe5ece2092e7b469ddded00b71ed1a4d4 100644 (file)
@@ -8,5 +8,5 @@ package main
 
 func main() {
        const a uint64 = 10
-       var b int64 = a // ERROR "convert|cannot|incompatible"
+       var _ int64 = a // ERROR "convert|cannot|incompatible"
 }
index 7e8346ee0f631bd8bc1383b2f5ffcdf7a1556f3f..868029a115a1d21a4002167336c2c6a3086a9b55 100644 (file)
@@ -10,11 +10,13 @@ package main
 
 func f1() {
        a, b := f() // ERROR "assignment mismatch|does not match|cannot initialize"
+       _, _ = a, b
 }
 
 func f2() {
        var a, b int
        a, b = f() // ERROR "assignment mismatch|does not match|cannot assign"
+       _, _ = a, b
 }
 
 func f() int {
index 062cb5ab955fba2fd57524c4fa37172f851ff4a9..75875c40041c4d5db4386b09227f575dccac88b6 100644 (file)
@@ -52,5 +52,5 @@ func g() {
 
        var t *T4
        t = i // ERROR "cannot use i \(variable of type I\) as \*T4 value in assignment: need type assertion"
-       _ = i
+       _ = t
 }
index ea53e7a69ae0b2c7c77a0e6cbb2942236bf3f61e..26d4d0f7656c50d2463caf99ef9b3097361ff64c 100644 (file)
@@ -13,6 +13,7 @@ const zero = 0
 
 func main() {
        var x int
+       _ = x
        x = make(map[int]int)       // ERROR "cannot use make\(map\[int\]int\)|incompatible"
        x = make(map[int]int, 0)    // ERROR "cannot use make\(map\[int\]int, 0\)|incompatible"
        x = make(map[int]int, zero) // ERROR "cannot use make\(map\[int\]int, zero\)|incompatible"
index a71b3f4bf89ddd2f048ec4913ea1cb91e0faebcb..c9651d2ce6d86b3f793029c6af37358650cc0871 100644 (file)
@@ -31,7 +31,7 @@ func AddInst(Inst) *Inst {
 
 func main() {
        print("call addinst\n")
-       var x Inst = AddInst(new(Start)) // ERROR "pointer to interface|incompatible type"
+       var _ Inst = AddInst(new(Start)) // ERROR "pointer to interface|incompatible type"
        print("return from  addinst\n")
-       var y *Inst = new(Start) // ERROR "pointer to interface|incompatible type"
+       var _ *Inst = new(Start) // ERROR "pointer to interface|incompatible type"
 }