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>

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.

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

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)

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)`,
 

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" */:
 		}
 	}
 }

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")