cmd/compile: avoid "not used" errors due to bad go/defer statements

The syntax for go and defer specifies an arbitrary expression, not
a call; the call requirement is spelled out in prose. Don't to the
call check in the parser; instead move it to the type checker. This
is simpler and also allows the type checker to check expressions that
are not calls, and avoid "not used" errors due to such expressions.

We would like to make the same change in go/parser and go/types
but the change requires Go/DeferStmt nodes to hold an ast.Expr
rather than an *ast.CallExpr. We cannot change that for backward-
compatibility reasons. Since we don't test this behavior for the
type checkers alone (only for the compiler), we get away with it
for now.

Follow-up on CL 425675 which introduced the extra errors in the
first place.

Change-Id: I90890b3079d249bdeeb76d5673246ba44bec1a7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/425794
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go
index d86fe1b72e43f31230c33236731c8a8d16a6350b..8ba72fe7cf841419363c710381c89f8c0e1a2ab9 100644 (file)
--- a/src/cmd/compile/internal/syntax/parser.go
+++ b/src/cmd/compile/internal/syntax/parser.go
@@ -953,20 +953,6 @@ func (p *parser) callStmt() *CallStmt {
                x = t
        }
 
-       // TODO(gri) Now that we don't store a CallExpr in a CallStmt anymore
-       //           we might as well leave this check to the type checker.
-       //           Adjust this here and in go/parser eventually.
-       if _, ok := x.(*CallExpr); !ok {
-               // only report an error if it's a new one
-               if bad, ok := x.(*BadExpr); !ok {
-                       p.errorAt(x.Pos(), fmt.Sprintf("expression in %s must be function call", s.Tok))
-                       // already progressed, no need to advance
-                       bad = new(BadExpr)
-                       bad.pos = x.Pos()
-                       x = bad
-               }
-       }
-
        s.Call = x
        return s
 }

diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go
index 92aa6b6f76827ed4331cac19f76e1f836e91135a..adb24d495dca3245026de5664dbd393091c6f2f7 100644 (file)
--- a/src/cmd/compile/internal/types2/stmt.go
+++ b/src/cmd/compile/internal/types2/stmt.go
@@ -166,6 +166,12 @@ func (check *Checker) closeScope() {
 }
 
 func (check *Checker) suspendedCall(keyword string, call syntax.Expr) {
+       if _, ok := call.(*syntax.CallExpr); !ok {
+               check.errorf(call, "expression in %s must be function call", keyword)
+               check.use(call)
+               return
+       }
+
        var x operand
        var msg string
        switch check.rawExpr(&x, call, nil, false) {

diff --git a/test/fixedbugs/issue23586.go b/test/fixedbugs/issue23586.go
index 31d39d83894c7251559f3d454f110ef9a2da2320..c2d4c9ffb59351996488f966f4ecf7dd51547f8f 100644 (file)
--- a/test/fixedbugs/issue23586.go
+++ b/test/fixedbugs/issue23586.go
@@ -11,15 +11,13 @@
 
 package p
 
-// TODO(gri) The "not used" errors should not be reported.
-
 import (
-       "fmt"  // ERROR "imported and not used"
-       "math" // ERROR "imported and not used"
+       "fmt"
+       "math"
 )
 
 func f() {
-       var i int // ERROR "i declared but not used"
+       var i int
        defer func() { fmt.Println() } // ERROR "must be function call"
        go func() { _ = math.Sin(0) }  // ERROR "must be function call"
        go func() { _ = i}             // ERROR "must be function call"