go/parser: remove validation of expression syntax, leave to type checker

Remove the code that verifies that an expression is a type or non-type
expression. For one, it cannot be done perfectly accurate
(e.g., consider *p which could be an indirection or a pointer type),
it also unnecessarily slows down parsing. It's simpler to leave the
verification to the type checker which has all the information needed.

Remove short compiler tests that tested the expression/type property.
Adjust a couple of go/types tests which now trigger because the parser
doesn't complain anymore.

Change file for benchmark from "parser.go" to "../printer/nodes.go"
to avoid a moving target when benchmarking.

The parser may be marginally faster when tested on nodes.go:

name          old time/op    new time/op    delta
ParseOnly-12    1.35ms ± 0%    1.31ms ± 0%   ~     (p=0.100 n=3+3)

name          old speed      new speed      delta
ParseOnly-12  39.9MB/s ± 0%  41.0MB/s ± 0%   ~     (p=0.100 n=3+3)

For #54511.

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

diff --git a/src/go/parser/interface.go b/src/go/parser/interface.go
index d911c8e1d0adb8d13d7774ee58c70f5b315ac41f..73cb16272e897d4c0db5b68c375d20a97621881c 100644 (file)
--- a/src/go/parser/interface.go
+++ b/src/go/parser/interface.go
@@ -214,7 +214,7 @@ func ParseExprFrom(fset *token.FileSet, filename string, src any, mode Mode) (ex
 
        // parse expr
        p.init(fset, filename, text, mode)
-       expr = p.parseRhsOrType()
+       expr = p.parseRhs()
 
        // If a semicolon was inserted, consume it;
        // report an error if there's more tokens.

diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go
index 3ac350d8f8cd9a6842774d7b1846430c3baf3173..3d4d83c4a41d9bb894a3c9eb5883812b6047ebea 100644 (file)
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -470,10 +470,10 @@ func (p *parser) parseExprList() (list []ast.Expr) {
                defer un(trace(p, "ExpressionList"))
        }
 
-       list = append(list, p.checkExpr(p.parseExpr()))
+       list = append(list, p.parseExpr())
        for p.tok == token.COMMA {
                p.next()
-               list = append(list, p.checkExpr(p.parseExpr()))
+               list = append(list, p.parseExpr())
        }
 
        return
@@ -581,10 +581,10 @@ func (p *parser) parseArrayFieldOrTypeInstance(x *ast.Ident) (*ast.Ident, ast.Ex
        var args []ast.Expr
        if p.tok != token.RBRACK {
                p.exprLev++
-               args = append(args, p.parseRhsOrType())
+               args = append(args, p.parseRhs())
                for p.tok == token.COMMA {
                        p.next()
-                       args = append(args, p.parseRhsOrType())
+                       args = append(args, p.parseRhs())
                }
                p.exprLev--
        }
@@ -1399,7 +1399,7 @@ func (p *parser) parseOperand() ast.Expr {
                lparen := p.pos
                p.next()
                p.exprLev++
-               x := p.parseRhsOrType() // types may be parenthesized: (some type)
+               x := p.parseRhs() // types may be parenthesized: (some type)
                p.exprLev--
                rparen := p.expect(token.RPAREN)
                return &ast.ParenExpr{Lparen: lparen, X: x, Rparen: rparen}
@@ -1478,7 +1478,7 @@ func (p *parser) parseIndexOrSliceOrInstance(x ast.Expr) ast.Expr {
        if p.tok != token.COLON {
                // We can't know if we have an index expression or a type instantiation;
                // so even if we see a (named) type we are not going to be in type context.
-               index[0] = p.parseRhsOrType()
+               index[0] = p.parseRhs()
        }
        ncolons := 0
        switch p.tok {
@@ -1544,7 +1544,7 @@ func (p *parser) parseCallOrConversion(fun ast.Expr) *ast.CallExpr {
        var list []ast.Expr
        var ellipsis token.Pos
        for p.tok != token.RPAREN && p.tok != token.EOF && !ellipsis.IsValid() {
-               list = append(list, p.parseRhsOrType()) // builtins may expect a type: make(some type, ...)
+               list = append(list, p.parseRhs()) // builtins may expect a type: make(some type, ...)
                if p.tok == token.ELLIPSIS {
                        ellipsis = p.pos
                        p.next()
@@ -1569,7 +1569,7 @@ func (p *parser) parseValue() ast.Expr {
                return p.parseLiteralValue(nil)
        }
 
-       x := p.checkExpr(p.parseExpr())
+       x := p.parseExpr()
 
        return x
 }
@@ -1621,38 +1621,6 @@ func (p *parser) parseLiteralValue(typ ast.Expr) ast.Expr {
        return &ast.CompositeLit{Type: typ, Lbrace: lbrace, Elts: elts, Rbrace: rbrace}
 }
 
-// checkExpr checks that x is an expression (and not a type).
-func (p *parser) checkExpr(x ast.Expr) ast.Expr {
-       switch unparen(x).(type) {
-       case *ast.BadExpr:
-       case *ast.Ident:
-       case *ast.BasicLit:
-       case *ast.FuncLit:
-       case *ast.CompositeLit:
-       case *ast.ParenExpr:
-               panic("unreachable")
-       case *ast.SelectorExpr:
-       case *ast.IndexExpr:
-       case *ast.IndexListExpr:
-       case *ast.SliceExpr:
-       case *ast.TypeAssertExpr:
-               // If t.Type == nil we have a type assertion of the form
-               // y.(type), which is only allowed in type switch expressions.
-               // It's hard to exclude those but for the case where we are in
-               // a type switch. Instead be lenient and test this in the type
-               // checker.
-       case *ast.CallExpr:
-       case *ast.StarExpr:
-       case *ast.UnaryExpr:
-       case *ast.BinaryExpr:
-       default:
-               // all other nodes are not proper expressions
-               p.errorExpected(x.Pos(), "expression")
-               x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())}
-       }
-       return x
-}
-
 // If x is of the form (T), unparen returns unparen(T), otherwise it returns x.
 func unparen(x ast.Expr) ast.Expr {
        if p, isParen := x.(*ast.ParenExpr); isParen {
@@ -1661,23 +1629,6 @@ func unparen(x ast.Expr) ast.Expr {
        return x
 }
 
-// checkExprOrType checks that x is an expression or a type
-// (and not a raw type such as [...]T).
-func (p *parser) checkExprOrType(x ast.Expr) ast.Expr {
-       switch t := unparen(x).(type) {
-       case *ast.ParenExpr:
-               panic("unreachable")
-       case *ast.ArrayType:
-               if len, isEllipsis := t.Len.(*ast.Ellipsis); isEllipsis {
-                       p.error(len.Pos(), "expected array length, found '...'")
-                       x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())}
-               }
-       }
-
-       // all other nodes are expressions or types
-       return x
-}
-
 func (p *parser) parsePrimaryExpr(x ast.Expr) ast.Expr {
        if p.trace {
                defer un(trace(p, "PrimaryExpr"))
@@ -1698,9 +1649,9 @@ func (p *parser) parsePrimaryExpr(x ast.Expr) ast.Expr {
                        p.next()
                        switch p.tok {
                        case token.IDENT:
-                               x = p.parseSelector(p.checkExprOrType(x))
+                               x = p.parseSelector(x)
                        case token.LPAREN:
-                               x = p.parseTypeAssertion(p.checkExpr(x))
+                               x = p.parseTypeAssertion(x)
                        default:
                                pos := p.pos
                                p.errorExpected(pos, "selector or type assertion")
@@ -1716,9 +1667,9 @@ func (p *parser) parsePrimaryExpr(x ast.Expr) ast.Expr {
                                x = &ast.SelectorExpr{X: x, Sel: sel}
                        }
                case token.LBRACK:
-                       x = p.parseIndexOrSliceOrInstance(p.checkExpr(x))
+                       x = p.parseIndexOrSliceOrInstance(x)
                case token.LPAREN:
-                       x = p.parseCallOrConversion(p.checkExprOrType(x))
+                       x = p.parseCallOrConversion(x)
                case token.LBRACE:
                        // operand may have returned a parenthesized complit
                        // type; accept it but complain if we have a complit
@@ -1763,7 +1714,7 @@ func (p *parser) parseUnaryExpr() ast.Expr {
                pos, op := p.pos, p.tok
                p.next()
                x := p.parseUnaryExpr()
-               return &ast.UnaryExpr{OpPos: pos, Op: op, X: p.checkExpr(x)}
+               return &ast.UnaryExpr{OpPos: pos, Op: op, X: x}
 
        case token.ARROW:
                // channel type or receive expression
@@ -1809,14 +1760,14 @@ func (p *parser) parseUnaryExpr() ast.Expr {
                }
 
                // <-(expr)
-               return &ast.UnaryExpr{OpPos: arrow, Op: token.ARROW, X: p.checkExpr(x)}
+               return &ast.UnaryExpr{OpPos: arrow, Op: token.ARROW, X: x}
 
        case token.MUL:
                // pointer type or unary "*" expression
                pos := p.pos
                p.next()
                x := p.parseUnaryExpr()
-               return &ast.StarExpr{Star: pos, X: p.checkExprOrType(x)}
+               return &ast.StarExpr{Star: pos, X: x}
        }
 
        return p.parsePrimaryExpr(nil)
@@ -1832,10 +1783,9 @@ func (p *parser) tokPrec() (token.Token, int) {
 
 // parseBinaryExpr parses a (possibly) binary expression.
 // If x is non-nil, it is used as the left operand.
-// If check is true, operands are checked to be valid expressions.
 //
 // TODO(rfindley): parseBinaryExpr has become overloaded. Consider refactoring.
-func (p *parser) parseBinaryExpr(x ast.Expr, prec1 int, check bool) ast.Expr {
+func (p *parser) parseBinaryExpr(x ast.Expr, prec1 int) ast.Expr {
        if p.trace {
                defer un(trace(p, "BinaryExpr"))
        }
@@ -1855,38 +1805,24 @@ func (p *parser) parseBinaryExpr(x ast.Expr, prec1 int, check bool) ast.Expr {
                        return x
                }
                pos := p.expect(op)
-               y := p.parseBinaryExpr(nil, oprec+1, check)
-               if check {
-                       x = p.checkExpr(x)
-                       y = p.checkExpr(y)
-               }
+               y := p.parseBinaryExpr(nil, oprec+1)
                x = &ast.BinaryExpr{X: x, OpPos: pos, Op: op, Y: y}
        }
 }
 
-// The result may be a type or even a raw type ([...]int). Callers must
-// check the result (using checkExpr or checkExprOrType), depending on
-// context.
+// The result may be a type or even a raw type ([...]int).
 func (p *parser) parseExpr() ast.Expr {
        if p.trace {
                defer un(trace(p, "Expression"))
        }
 
-       return p.parseBinaryExpr(nil, token.LowestPrec+1, true)
+       return p.parseBinaryExpr(nil, token.LowestPrec+1)
 }
 
 func (p *parser) parseRhs() ast.Expr {
        old := p.inRhs
        p.inRhs = true
-       x := p.checkExpr(p.parseExpr())
-       p.inRhs = old
-       return x
-}
-
-func (p *parser) parseRhsOrType() ast.Expr {
-       old := p.inRhs
-       p.inRhs = true
-       x := p.checkExprOrType(p.parseExpr())
+       x := p.parseExpr()
        p.inRhs = old
        return x
 }
@@ -1991,7 +1927,7 @@ func (p *parser) checkAssignStmt(as *ast.AssignStmt) {
 }
 
 func (p *parser) parseCallExpr(callType string) *ast.CallExpr {
-       x := p.parseRhsOrType() // could be a conversion: (some type)(x)
+       x := p.parseRhs() // could be a conversion: (some type)(x)
        if call, isCall := x.(*ast.CallExpr); isCall {
                return call
        }
@@ -2068,7 +2004,7 @@ func (p *parser) makeExpr(s ast.Stmt, want string) ast.Expr {
                return nil
        }
        if es, isExpr := s.(*ast.ExprStmt); isExpr {
-               return p.checkExpr(es.X)
+               return es.X
        }
        found := "simple statement"
        if _, isAss := s.(*ast.AssignStmt); isAss {
@@ -2173,21 +2109,7 @@ func (p *parser) parseIfStmt() *ast.IfStmt {
        return &ast.IfStmt{If: pos, Init: init, Cond: cond, Body: body, Else: else_}
 }
 
-func (p *parser) parseTypeList() (list []ast.Expr) {
-       if p.trace {
-               defer un(trace(p, "TypeList"))
-       }
-
-       list = append(list, p.parseType())
-       for p.tok == token.COMMA {
-               p.next()
-               list = append(list, p.parseType())
-       }
-
-       return
-}
-
-func (p *parser) parseCaseClause(typeSwitch bool) *ast.CaseClause {
+func (p *parser) parseCaseClause() *ast.CaseClause {
        if p.trace {
                defer un(trace(p, "CaseClause"))
        }
@@ -2196,11 +2118,7 @@ func (p *parser) parseCaseClause(typeSwitch bool) *ast.CaseClause {
        var list []ast.Expr
        if p.tok == token.CASE {
                p.next()
-               if typeSwitch {
-                       list = p.parseTypeList()
-               } else {
-                       list = p.parseList(true)
-               }
+               list = p.parseList(true)
        } else {
                p.expect(token.DEFAULT)
        }
@@ -2278,7 +2196,7 @@ func (p *parser) parseSwitchStmt() ast.Stmt {
        lbrace := p.expect(token.LBRACE)
        var list []ast.Stmt
        for p.tok == token.CASE || p.tok == token.DEFAULT {
-               list = append(list, p.parseCaseClause(typeSwitch))
+               list = append(list, p.parseCaseClause())
        }
        rbrace := p.expect(token.RBRACE)
        p.expectSemi()
@@ -2643,7 +2561,7 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
                                // to parser.expr, and pass in name to parsePrimaryExpr.
                                p.exprLev++
                                lhs := p.parsePrimaryExpr(x)
-                               x = p.parseBinaryExpr(lhs, token.LowestPrec+1, false)
+                               x = p.parseBinaryExpr(lhs, token.LowestPrec+1)
                                p.exprLev--
                        }
                        // Analyze expression x. If we can split x into a type parameter

diff --git a/src/go/parser/performance_test.go b/src/go/parser/performance_test.go
index 1249f35d39da186efd7367c04801b7f68d90b731..1308f212dc0cb6ed2f1e9257c393339152b427fa 100644 (file)
--- a/src/go/parser/performance_test.go
+++ b/src/go/parser/performance_test.go
@@ -10,9 +10,7 @@ import (
        "testing"
 )
 
-// TODO(rfindley): use a testdata file or file from another package here, to
-// avoid a moving target.
-var src = readFile("parser.go")
+var src = readFile("../printer/nodes.go")
 
 func readFile(filename string) []byte {
        data, err := os.ReadFile(filename)

diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go
index 86779e7e7e93ef1d35f7ea02fc18de72ad4e16c7..2d9016aaddc7dd90bb6d559b510fbd8655d95bad 100644 (file)
--- a/src/go/parser/short_test.go
+++ b/src/go/parser/short_test.go
@@ -141,18 +141,6 @@ var invalids = []string{
        `package p; func f() { switch t = /* ERROR "expected ':=', found '='" */ t.(type) {} };`,
        `package p; func f() { switch t /* ERROR "expected switch expression" */ , t = t.(type) {} };`,
        `package p; func f() { switch t /* ERROR "expected switch expression" */ = t.(type), t {} };`,
-       `package p; var a = [ /* ERROR "expected expression" */ 1]int;`,
-       `package p; var a = [ /* ERROR "expected expression" */ ...]int;`,
-       `package p; var a = struct /* ERROR "expected expression" */ {}`,
-       `package p; var a = func /* ERROR "expected expression" */ ();`,
-       `package p; var a = interface /* ERROR "expected expression" */ {}`,
-       `package p; var a = [ /* ERROR "expected expression" */ ]int`,
-       `package p; var a = map /* ERROR "expected expression" */ [int]int`,
-       `package p; var a = chan /* ERROR "expected expression" */ int;`,
-       `package p; var a = []int{[ /* ERROR "expected expression" */ ]int};`,
-       `package p; var a = ( /* ERROR "expected expression" */ []int);`,
-       `package p; var a = <- /* ERROR "expected expression" */ chan int;`,
-       `package p; func f() { select { case _ <- chan /* ERROR "expected expression" */ int: } };`,
        `package p; func f() { _ = (<-<- /* ERROR "expected 'chan'" */ chan int)(nil) };`,
        `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan<- /* ERROR "expected channel type" */ int)(nil) };`,
        `package p; func f() { var t []int; t /* ERROR "expected identifier on left side of :=" */ [0] := 0 };`,
@@ -183,13 +171,6 @@ var invalids = []string{
        `package p; type _ struct{ *( /* ERROR "cannot parenthesize embedded type" */ int) }`,
        `package p; type _ struct{ *( /* ERROR "cannot parenthesize embedded type" */ []byte) }`,
 
-       // TODO(rfindley): this error should be positioned on the ':'
-       `package p; var a = a[[]int:[ /* ERROR "expected expression" */ ]int];`,
-
-       // TODO(rfindley): the compiler error is better here: "cannot parenthesize embedded type"
-       // TODO(rfindley): confirm that parenthesized types should now be accepted.
-       // `package p; type I1 interface{}; type I2 interface{ (/* ERROR "expected '}', found '\('" */ I1) }`,
-
        // issue 8656
        `package p; func f() (a b string /* ERROR "missing ','" */ , ok bool)`,
 

diff --git a/src/go/types/testdata/check/stmt0.go b/src/go/types/testdata/check/stmt0.go
index d8790b9616da834093146b67148e2e68bed484aa..0caebcf544fced402fc5aecec5c2cf719b042b25 100644 (file)
--- a/src/go/types/testdata/check/stmt0.go
+++ b/src/go/types/testdata/check/stmt0.go
@@ -728,7 +728,7 @@ func typeswitches() {
                case int:
                        println(x)
                        println(x / 0 /* ERROR "invalid operation: division by zero" */)
-               case 1 /* ERROR "expected type, found 1" */:
+               case 1 /* ERROR "1 is not a type" */:
                }
        }
 }

diff --git a/src/go/types/testdata/fixedbugs/issue42987.go b/src/go/types/testdata/fixedbugs/issue42987.go
index 6060ec84bde715837cf141b8b5787d2f6e48733f..f58c63f8a3621b85f581bf1f4600b6ff4ede76e3 100644 (file)
--- a/src/go/types/testdata/fixedbugs/issue42987.go
+++ b/src/go/types/testdata/fixedbugs/issue42987.go
@@ -5,6 +5,4 @@
 // Check that there is only one error (no follow-on errors).
 
 package p
-// TODO(rFindley) This is a parser error, but in types2 it is a type checking
-//                error. We could probably do without this check in the parser.
-var _ = [... /* ERROR expected array length, found '...' */ ]byte("foo")
+var _ = [ ... /* ERROR invalid use of \[...\] array */ ]byte("foo")
\ No newline at end of file