From 0fb733b7f79001092897282749bf5942953b0675 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Wed, 18 Nov 2020 15:02:58 -0500 Subject: [PATCH] [dev.typeparams] go/parser: support the ParseTypeParams mode Support is added for parsing type parameters only if the ParseTypeParams mode is set, otherwise emitting syntax errors for source code that is invalid without type parameters. Rather than have large conditional blocks switching between legacy parser logic and new parser logic, effort is made to minimize special handling for ParseTypeParams. Change-Id: I243f6c4b9b8eb1313b838e8649b6cc1e5e8339ba Reviewed-on: https://go-review.googlesource.com/c/go/+/271218 Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer Trust: Robert Griesemer Trust: Robert Findley --- src/go/parser/error_test.go | 13 ++- src/go/parser/parser.go | 51 ++++++--- src/go/parser/short_test.go | 222 ++++++++++++++++++++---------------- 3 files changed, 167 insertions(+), 119 deletions(-) diff --git a/src/go/parser/error_test.go b/src/go/parser/error_test.go index ed9e9473daf..83bfdd40adc 100644 --- a/src/go/parser/error_test.go +++ b/src/go/parser/error_test.go @@ -150,7 +150,7 @@ func compareErrors(t *testing.T, fset *token.FileSet, expected map[token.Pos]str } } -func checkErrors(t *testing.T, filename string, input interface{}, mode Mode) { +func checkErrors(t *testing.T, filename string, input interface{}, mode Mode, expectErrors bool) { t.Helper() src, err := readSource(filename, input) if err != nil { @@ -167,9 +167,12 @@ func checkErrors(t *testing.T, filename string, input interface{}, mode Mode) { } found.RemoveMultiples() - // we are expecting the following errors - // (collect these after parsing a file so that it is found in the file set) - expected := expectedErrors(fset, filename, src) + expected := map[token.Pos]string{} + if expectErrors { + // we are expecting the following errors + // (collect these after parsing a file so that it is found in the file set) + expected = expectedErrors(fset, filename, src) + } // verify errors returned by the parser compareErrors(t, fset, expected, found) @@ -187,7 +190,7 @@ func TestErrors(t *testing.T) { if strings.HasSuffix(name, ".go2") { mode |= ParseTypeParams } - checkErrors(t, filepath.Join(testdata, name), nil, mode) + checkErrors(t, filepath.Join(testdata, name), nil, mode, true) } } } diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index 6d92373c33d..9c414c411e0 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -651,7 +651,7 @@ func (p *parser) parseQualifiedIdent(ident *ast.Ident) ast.Expr { } typ := p.parseTypeName(ident) - if p.tok == token.LBRACK { + if p.tok == token.LBRACK && p.mode&ParseTypeParams != 0 { typ = p.parseTypeInstance(typ) } @@ -708,12 +708,22 @@ func (p *parser) parseArrayFieldOrTypeInstance(x *ast.Ident) (*ast.Ident, ast.Ex // list such as T[P,]? (We do in parseTypeInstance). lbrack := p.expect(token.LBRACK) var args []ast.Expr + var firstComma token.Pos + // TODO(rfindley): consider changing parseRhsOrType so that this function variable + // is not needed. + argparser := p.parseRhsOrType + if p.mode&ParseTypeParams == 0 { + argparser = p.parseRhs + } if p.tok != token.RBRACK { p.exprLev++ - args = append(args, p.parseRhsOrType()) + args = append(args, argparser()) for p.tok == token.COMMA { + if !firstComma.IsValid() { + firstComma = p.pos + } p.next() - args = append(args, p.parseRhsOrType()) + args = append(args, argparser()) } p.exprLev-- } @@ -732,6 +742,15 @@ func (p *parser) parseArrayFieldOrTypeInstance(x *ast.Ident) (*ast.Ident, ast.Ex // x [P]E return x, &ast.ArrayType{Lbrack: lbrack, Len: args[0], Elt: elt} } + if p.mode&ParseTypeParams == 0 { + p.error(rbrack, "missing element type in array type expression") + return nil, &ast.BadExpr{From: args[0].Pos(), To: args[0].End()} + } + } + + if p.mode&ParseTypeParams == 0 { + p.error(firstComma, "expected ']', found ','") + return x, &ast.BadExpr{From: args[0].Pos(), To: args[len(args)-1].End()} } // x[P], x[P1, P2], ... @@ -1025,7 +1044,7 @@ func (p *parser) parseParameters(scope *ast.Scope, acceptTParams bool) (tparams, defer un(trace(p, "Parameters")) } - if acceptTParams && p.tok == token.LBRACK { + if p.mode&ParseTypeParams != 0 && acceptTParams && p.tok == token.LBRACK { opening := p.pos p.next() // [T any](params) syntax @@ -1098,8 +1117,8 @@ func (p *parser) parseMethodSpec(scope *ast.Scope) *ast.Field { var typ ast.Expr x := p.parseTypeName(nil) if ident, _ := x.(*ast.Ident); ident != nil { - switch p.tok { - case token.LBRACK: + switch { + case p.tok == token.LBRACK && p.mode&ParseTypeParams != 0: // generic method or embedded instantiated type lbrack := p.pos p.next() @@ -1135,7 +1154,7 @@ func (p *parser) parseMethodSpec(scope *ast.Scope) *ast.Field { rbrack := p.expectClosing(token.RBRACK, "type argument list") typ = &ast.CallExpr{Fun: ident, Lparen: lbrack, Args: list, Rparen: rbrack, Brackets: true} } - case token.LPAREN: + case p.tok == token.LPAREN: // ordinary method // TODO(rfindley) refactor to share code with parseFuncType. scope := ast.NewScope(nil) // method scope @@ -1151,7 +1170,7 @@ func (p *parser) parseMethodSpec(scope *ast.Scope) *ast.Field { } else { // embedded, possibly instantiated type typ = x - if p.tok == token.LBRACK { + if p.tok == token.LBRACK && p.mode&ParseTypeParams != 0 { // embedded instantiated interface typ = p.parseTypeInstance(typ) } @@ -1173,12 +1192,10 @@ func (p *parser) parseInterfaceType() *ast.InterfaceType { lbrace := p.expect(token.LBRACE) scope := ast.NewScope(nil) // interface scope var list []*ast.Field -L: - for { - switch p.tok { - case token.IDENT, token.LPAREN: + for p.tok == token.IDENT || p.mode&ParseTypeParams != 0 && p.tok == token.TYPE { + if p.tok == token.IDENT { list = append(list, p.parseMethodSpec(scope)) - case token.TYPE: + } else { // all types in a type list share the same field name "type" // (since type is a keyword, a Go program cannot have that field name) name := []*ast.Ident{{NamePos: p.pos, Name: "type"}} @@ -1188,10 +1205,10 @@ L: list = append(list, &ast.Field{Names: name, Type: typ}) } p.expectSemi() - default: - break L } } + // TODO(rfindley): the error produced here could be improved, since we could + // accept a identifier, 'type', or a '}' at this point. rbrace := p.expect(token.RBRACE) return &ast.InterfaceType{ @@ -1271,7 +1288,7 @@ func (p *parser) tryIdentOrType() ast.Expr { switch p.tok { case token.IDENT: typ := p.parseTypeName(nil) - if p.tok == token.LBRACK { + if p.tok == token.LBRACK && p.mode&ParseTypeParams != 0 { typ = p.parseTypeInstance(typ) } return typ @@ -2668,7 +2685,7 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token p.exprLev++ x := p.parseExpr(true) // we don't know yet if we're a lhs or rhs expr p.exprLev-- - if name0, _ := x.(*ast.Ident); name0 != nil && p.tok != token.RBRACK { + if name0, _ := x.(*ast.Ident); p.mode&ParseTypeParams != 0 && name0 != nil && p.tok != token.RBRACK { // generic type [T any]; p.parseGenericType(spec, lbrack, name0, token.RBRACK) } else { diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go index 46fa764466b..3676c275597 100644 --- a/src/go/parser/short_test.go +++ b/src/go/parser/short_test.go @@ -49,103 +49,100 @@ var valids = []string{ `package p; type T = int`, `package p; type (T = p.T; _ = struct{}; x = *T)`, `package p; type T (*int)`, - - // structs with parameterized embedded fields (for symmetry with interfaces) `package p; type _ struct{ ((int)) }`, `package p; type _ struct{ (*(int)) }`, `package p; type _ struct{ ([]byte) }`, // disallowed by type-checker - - // type parameters - `package p; type T[P any] struct { P }`, - `package p; type T[P comparable] struct { P }`, - `package p; type T[P comparable[P]] struct { P }`, - `package p; type T[P1, P2 any] struct { P1; f []P2 }`, - `package p; type _ []T[int]`, - `package p; var _ = func()T(nil)`, - `package p; func _[T any]()`, - `package p; func _[T any]()()`, `package p; func _(T (P))`, `package p; func _(T []E)`, `package p; func _(T [P]E)`, - `package p; func _(x T[P1, P2, P3])`, - `package p; func _(x p.T[Q])`, - `package p; func _(p.T[Q])`, - - `package p; var _ T[chan int]`, - `package p; func f[A, B any](); func _() { _ = f[int, int] }`, - - `package p; type _[A interface{},] struct{}`, - `package p; type _[A interface{}] struct{}`, - `package p; type _[A, B any,] struct{}`, - `package p; type _[A, B any] struct{}`, - `package p; type _[A any,] struct{}`, - `package p; type _ [A+B]struct{}`, // this is an array! - `package p; type _[A any]struct{}`, - `package p; type _[A any] struct{ A }`, // this is not an array! - - `package p; func _[T any]()`, - `package p; func _[T any](x T)`, - `package p; func _[T1, T2 any](x T)`, - + `package p; type _ [A+B]struct{}`, `package p; func (R) _()`, - `package p; func (R[P]) _[T any]()`, - `package p; func (_ R[P]) _[T any](x T)`, - `package p; func (_ R[P, Q]) _[T1, T2 any](x T)`, - - `package p; var _ = []T[int]{}`, - `package p; var _ = [10]T[int]{}`, - `package p; var _ = func()T[int]{}`, - `package p; var _ = map[T[int]]T[int]{}`, - `package p; var _ = chan T[int](x)`, - `package p; func _(T[P])`, - `package p; func _(T[P1, P2, P3])`, - `package p; func _(T[P]) T[P]`, - `package p; func _(_ T[P], T P) T[P]`, - - `package p; func _[A, B any](a A) B`, - `package p; func _[A, B C](a A) B`, - `package p; func _[A, B C[A, B]](a A) B`, - - // method type parameters (if methodTypeParamsOk) - `package p; func (T) _[A, B any](a A) B`, - `package p; func (T) _[A, B C](a A) B`, - `package p; func (T) _[A, B C[A, B]](a A) B`, - - // method type parameters are not permitted in interfaces. - `package p; type _[A, B any] interface { _(a A) B }`, - `package p; type _[A, B C[A, B]] interface { _(a A) B }`, - - // type bounds - `package p; func _[T1, T2 interface{}](x T1) T2`, - `package p; func _[T1 interface{ m() }, T2, T3 interface{}](x T1, y T3) T2`, - - // struct embedding - `package p; type _ struct{ T[P] }`, - `package p; type _ struct{ T[struct{a, b, c int}] }`, `package p; type _ struct{ f [n]E }`, `package p; type _ struct{ f [a+b+c+d]E }`, - - // interfaces with type lists - `package p; type _ interface{type int}`, - `package p; type _ interface{type int, float32; type bool; m(); type string;}`, - - // interface embedding `package p; type I1 interface{}; type I2 interface{ I1 }`, - `package p; type I1[T any] interface{}; type I2 interface{ I1[int] }`, - `package p; type I1[T any] interface{}; type I2[T any] interface{ I1[T] }`, +} + +// validWithTParamsOnly holds source code examples that are valid if +// ParseTypeParams is set, but invalid if not. When checking with the +// ParseTypeParams set, errors are ignored. +var validWithTParamsOnly = []string{ + `package p; type _ []T[ /* ERROR "expected ';', found '\['" */ int]`, + `package p; type T[P any /* ERROR "expected ']', found any" */ ] struct { P }`, + `package p; type T[P comparable /* ERROR "expected ']', found comparable" */ ] struct { P }`, + `package p; type T[P comparable /* ERROR "expected ']', found comparable" */ [P]] struct { P }`, + `package p; type T[P1, /* ERROR "expected ']', found ','" */ P2 any] struct { P1; f []P2 }`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T any]()()`, + `package p; func _(T (P))`, + `package p; func f[ /* ERROR "expected '\(', found '\['" */ A, B any](); func _() { _ = f[int, int] }`, + `package p; func _(x /* ERROR "mixed named and unnamed parameters" */ T[P1, P2, P3])`, + `package p; func _(x /* ERROR "mixed named and unnamed parameters" */ p.T[Q])`, + `package p; func _(p.T[ /* ERROR "missing ',' in parameter list" */ Q])`, + `package p; type _[A interface /* ERROR "expected ']', found 'interface'" */ {},] struct{}`, + `package p; type _[A interface /* ERROR "expected ']', found 'interface'" */ {}] struct{}`, + `package p; type _[A, /* ERROR "expected ']', found ','" */ B any,] struct{}`, + `package p; type _[A, /* ERROR "expected ']', found ','" */ B any] struct{}`, + `package p; type _[A any /* ERROR "expected ']', found any" */,] struct{}`, + `package p; type _[A any /* ERROR "expected ']', found any" */ ]struct{}`, + `package p; type _[A any /* ERROR "expected ']', found any" */ ] struct{ A }`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T any]()`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T any](x T)`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T1, T2 any](x T)`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ A, B any](a A) B`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ A, B C](a A) B`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ A, B C[A, B]](a A) B`, + `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B any](a A) B`, + `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B C](a A) B`, + `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B C[A, B]](a A) B`, + `package p; type _[A, /* ERROR "expected ']', found ','" */ B any] interface { _(a A) B }`, + `package p; type _[A, /* ERROR "expected ']', found ','" */ B C[A, B]] interface { _(a A) B }`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T1, T2 interface{}](x T1) T2`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ T1 interface{ m() }, T2, T3 interface{}](x T1, y T3) T2`, + `package p; var _ = [ /* ERROR "expected expression" */ ]T[int]{}`, + `package p; var _ = [ /* ERROR "expected expression" */ 10]T[int]{}`, + `package p; var _ = func /* ERROR "expected expression" */ ()T[int]{}`, + `package p; var _ = map /* ERROR "expected expression" */ [T[int]]T[int]{}`, + `package p; var _ = chan /* ERROR "expected expression" */ T[int](x)`, + `package p; func _(_ T[ /* ERROR "missing ',' in parameter list" */ P], T P) T[P]`, + `package p; var _ T[ /* ERROR "expected ';', found '\['" */ chan int]`, + + // TODO(rfindley) this error message could be improved. + `package p; func (_ /* ERROR "mixed named and unnamed parameters" */ R[P]) _[T any](x T)`, + `package p; func (_ /* ERROR "mixed named and unnamed parameters" */ R[ P, Q]) _[T1, T2 any](x T)`, + + `package p; func (R[P] /* ERROR "missing element type" */ ) _[T any]()`, + `package p; func _(T[P] /* ERROR "missing element type" */ )`, + `package p; func _(T[P1, /* ERROR "expected ']', found ','" */ P2, P3 ])`, + `package p; func _(T[P] /* ERROR "missing element type" */ ) T[P]`, + `package p; type _ struct{ T[P] /* ERROR "missing element type" */ }`, + `package p; type _ struct{ T[struct /* ERROR "expected expression" */ {a, b, c int}] }`, + `package p; type _ interface{type /* ERROR "expected '}', found 'type'" */ int}`, + `package p; type _ interface{type /* ERROR "expected '}', found 'type'" */ int, float32; type bool; m(); type string;}`, + `package p; type I1[T any /* ERROR "expected ']', found any" */ ] interface{}; type I2 interface{ I1[int] }`, + `package p; type I1[T any /* ERROR "expected ']', found any" */ ] interface{}; type I2[T any] interface{ I1[T] }`, + `package p; type _ interface { f[ /* ERROR "expected ';', found '\['" */ T any]() }`, } func TestValid(t *testing.T) { - for _, src := range valids { - checkErrors(t, src, src, DeclarationErrors|AllErrors) - } + t.Run("no tparams", func(t *testing.T) { + for _, src := range valids { + checkErrors(t, src, src, DeclarationErrors|AllErrors, false) + } + }) + t.Run("tparams", func(t *testing.T) { + for _, src := range valids { + checkErrors(t, src, src, DeclarationErrors|AllErrors|ParseTypeParams, false) + } + for _, src := range validWithTParamsOnly { + checkErrors(t, src, src, DeclarationErrors|AllErrors|ParseTypeParams, false) + } + }) } // TestSingle is useful to track down a problem with a single short test program. func TestSingle(t *testing.T) { const src = `package p; var _ = T[P]{}` - checkErrors(t, src, src, DeclarationErrors|AllErrors) + checkErrors(t, src, src, DeclarationErrors|AllErrors|ParseTypeParams, true) } var invalids = []string{ @@ -193,22 +190,14 @@ var invalids = []string{ `package p; func f() { go f /* ERROR HERE "function must be invoked" */ }`, `package p; func f() { defer func() {} /* ERROR HERE "function must be invoked" */ }`, `package p; func f() { go func() { func() { f(x func /* ERROR "missing ','" */ (){}) } } }`, - //`package p; func f(x func(), u v func /* ERROR "missing ','" */ ()){}`, - - // type parameters - `package p; var _ func[ /* ERROR "cannot have type parameters" */ T any](T)`, `package p; func _() (type /* ERROR "found 'type'" */ T)(T)`, `package p; func (type /* ERROR "found 'type'" */ T)(T) _()`, `package p; type _[A+B, /* ERROR "expected ']'" */ ] int`, - `package p; type _[_ any] int; var _ = T[] /* ERROR "expected operand" */ {}`, - `package p; type T[P any] = /* ERROR "cannot be alias" */ T0`, - `package p; func _[]/* ERROR "empty type parameter list" */()`, - // errors that could be improved - `package p; var a = a[[]int:[ /* ERROR "expected expression" */ ]int];`, // TODO: should this be on the ':'? - `package p; type _[A/* ERROR "all type parameters must be named" */,] struct{ A }`, // TODO: a better location would be after the ']' - `package p; func _[type /* ERROR "all type parameters must be named" */P, *Q interface{}]()`, // TODO: this is confusing. - `package p; type I1 interface{}; type I2 interface{ (/* ERROR "expected 'IDENT'" */I1) }`, // TODO: compiler error is 'syntax error: cannot parenthesize embedded type' + // TODO: this error should be positioned on the ':' + `package p; var a = a[[]int:[ /* ERROR "expected expression" */ ]int];`, + // TODO: the compiler error is better here: "cannot parenthesize embedded type" + `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)`, @@ -226,17 +215,56 @@ var invalids = []string{ // issue 11611 `package p; type _ struct { int, } /* ERROR "expected 'IDENT', found '}'" */ ;`, `package p; type _ struct { int, float } /* ERROR "expected type, found '}'" */ ;`, - //`package p; type _ struct { ( /* ERROR "cannot parenthesize embedded type" */ int) };`, - //`package p; func _()(x, y, z ... /* ERROR "expected '\)', found '...'" */ int){}`, - //`package p; func _()(... /* ERROR "expected type, found '...'" */ int){}`, // issue 13475 `package p; func f() { if true {} else ; /* ERROR "expected if statement or block" */ }`, `package p; func f() { if true {} else defer /* ERROR "expected if statement or block" */ f() }`, } -func TestInvalid(t *testing.T) { - for _, src := range invalids { - checkErrors(t, src, src, DeclarationErrors|AllErrors) - } +// invalidNoTParamErrs holds invalid source code examples annotated with the +// error messages produced when ParseTypeParams is not set. +var invalidNoTParamErrs = []string{ + `package p; type _[_ any /* ERROR "expected ']', found any" */ ] int; var _ = T[]{}`, + `package p; type T[P any /* ERROR "expected ']', found any" */ ] = T0`, + `package p; var _ func[ /* ERROR "expected '\(', found '\['" */ T any](T)`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ ]()`, + `package p; type _[A, /* ERROR "expected ']', found ','" */] struct{ A }`, + `package p; func _[ /* ERROR "expected '\(', found '\['" */ type P, *Q interface{}]()`, +} + +// invalidTParamErrs holds invalid source code examples annotated with the +// error messages produced when ParseTypeParams is set. +var invalidTParamErrs = []string{ + `package p; type _[_ any] int; var _ = T[] /* ERROR "expected operand" */ {}`, + `package p; type T[P any] = /* ERROR "cannot be alias" */ T0`, + `package p; var _ func[ /* ERROR "cannot have type parameters" */ T any](T)`, + `package p; func _[]/* ERROR "empty type parameter list" */()`, + + // TODO(rfindley) a better location would be after the ']' + `package p; type _[A/* ERROR "all type parameters must be named" */,] struct{ A }`, + + // TODO(rfindley) this error is confusing. + `package p; func _[type /* ERROR "all type parameters must be named" */P, *Q interface{}]()`, +} + +func TestInvalid(t *testing.T) { + t.Run("no tparams", func(t *testing.T) { + for _, src := range invalids { + checkErrors(t, src, src, DeclarationErrors|AllErrors, true) + } + for _, src := range validWithTParamsOnly { + checkErrors(t, src, src, DeclarationErrors|AllErrors, true) + } + for _, src := range invalidNoTParamErrs { + checkErrors(t, src, src, DeclarationErrors|AllErrors, true) + } + }) + t.Run("tparams", func(t *testing.T) { + for _, src := range invalids { + checkErrors(t, src, src, DeclarationErrors|AllErrors|ParseTypeParams, true) + } + for _, src := range invalidTParamErrs { + checkErrors(t, src, src, DeclarationErrors|AllErrors|ParseTypeParams, true) + } + }) }