[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 <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>

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
+	expected := map[token.Pos]string{}
-	// (collect these after parsing a file so that it is found in the file set)
+	if expectErrors {
-	expected := expectedErrors(fset, filename, src)
+		// 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)
 		}
 	}
 }

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 {
+		switch {
-		case token.LBRACK:
+		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 p.tok == token.IDENT || p.mode&ParseTypeParams != 0 && p.tok == token.TYPE {
-	for {
+		if p.tok == token.IDENT {
-		switch p.tok {
-		case token.IDENT, token.LPAREN:
 			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 {

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; type _ [A+B]struct{}`,
-	`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; 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 {
+	t.Run("no tparams", func(t *testing.T) {
-		checkErrors(t, src, src, DeclarationErrors|AllErrors)
+		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
+	// TODO: this error should be positioned on the ':'
-	`package p; var a = a[[]int:[ /* ERROR "expected expression" */ ]int];`,                      // TODO: should this be on the ':'?
+	`package p; var a = a[[]int:[ /* ERROR "expected expression" */ ]int];`,
-	`package p; type _[A/* ERROR "all type parameters must be named" */,] struct{ A }`,           // TODO: a better location would be after the ']'
+	// TODO: the compiler error is better here: "cannot parenthesize embedded type"
-	`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 '}', found '\('" */ I1) }`,
-	`package p; type I1 interface{}; type I2 interface{ (/* ERROR "expected 'IDENT'" */I1) }`,    // TODO: compiler error is 'syntax error: cannot parenthesize embedded type'
 
 	// 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) {
+// invalidNoTParamErrs holds invalid source code examples annotated with the
-	for _, src := range invalids {
+// error messages produced when ParseTypeParams is not set.
-		checkErrors(t, src, src, DeclarationErrors|AllErrors)
+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)
+		}
+	})
 }