From 2ecdf9d800f631cfde30b7463a3ed2c0b60611d5 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 8 Oct 2021 16:28:35 -0400 Subject: [PATCH] go/parser: allow eliding interface in constraint literals This is a port of CL 353133 from cmd/compile/internal/syntax, with significant adjustments for the mechanics of go/parser. Some additional cleanup is made along the way: parseParameterList can call parseParamDecl without indirection, and the tparams argument is redundant with the closing token. Also, the error that "all type parameters must be named" is positioned on the first unnamed type parameter. Error recovery in go/parser is notably worse here than the compiler parser, so the test data had to be adjusted to synchronize positions. Fixing this error recovery will have to wait for a later CL. As with the compiler changes, these changes are guarded behind a flag so that they may be easily removed if #48424 is not accepted. For #48424 Change-Id: If87925d246f36aaab11a95442f75f659462d4286 Reviewed-on: https://go-review.googlesource.com/c/go/+/354870 Trust: Robert Findley Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/internal/typeparams/common.go | 10 +- src/go/parser/parser.go | 126 +++++++++++++++--------- src/go/parser/testdata/typeset.go2 | 75 ++++++++++++++ src/go/printer/testdata/generics.golden | 5 + src/go/printer/testdata/generics.input | 5 + 5 files changed, 170 insertions(+), 51 deletions(-) create mode 100644 src/go/parser/testdata/typeset.go2 diff --git a/src/go/internal/typeparams/common.go b/src/go/internal/typeparams/common.go index 47b8f7cf02..9b82e6061a 100644 --- a/src/go/internal/typeparams/common.go +++ b/src/go/internal/typeparams/common.go @@ -7,7 +7,9 @@ // constraint. package typeparams -// DisallowParsing is the numeric value of a parsing mode that disallows type -// parameters. This only matters if the typeparams experiment is active, and -// may be used for running tests that disallow generics. -const DisallowParsing = 1 << 30 +// 'Hidden' parser modes to control the parsing of type-parameter related +// features. +const ( + DisallowTypeSets = 1 << 29 // Disallow eliding 'interface' in constraint type sets. + DisallowParsing = 1 << 30 // Disallow type parameters entirely. +) diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index dd6b93d20f..4f7a498780 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -76,9 +76,8 @@ func (p *parser) init(fset *token.FileSet, filename string, src []byte, mode Mod p.next() } -func (p *parser) parseTypeParams() bool { - return p.mode&typeparams.DisallowParsing == 0 -} +func (p *parser) allowGenerics() bool { return p.mode&typeparams.DisallowParsing == 0 } +func (p *parser) allowTypeSets() bool { return p.mode&typeparams.DisallowTypeSets == 0 } // ---------------------------------------------------------------------------- // Parsing support @@ -499,7 +498,7 @@ func (p *parser) parseQualifiedIdent(ident *ast.Ident) ast.Expr { } typ := p.parseTypeName(ident) - if p.tok == token.LBRACK && p.parseTypeParams() { + if p.tok == token.LBRACK && p.allowGenerics() { typ = p.parseTypeInstance(typ) } @@ -558,7 +557,7 @@ func (p *parser) parseArrayFieldOrTypeInstance(x *ast.Ident) (*ast.Ident, ast.Ex // TODO(rfindley): consider changing parseRhsOrType so that this function variable // is not needed. argparser := p.parseRhsOrType - if !p.parseTypeParams() { + if !p.allowGenerics() { argparser = p.parseRhs } if p.tok != token.RBRACK { @@ -588,13 +587,13 @@ 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.parseTypeParams() { + if !p.allowGenerics() { p.error(rbrack, "missing element type in array type expression") return nil, &ast.BadExpr{From: args[0].Pos(), To: args[0].End()} } } - if !p.parseTypeParams() { + if !p.allowGenerics() { p.error(firstComma, "expected ']', found ','") return x, &ast.BadExpr{From: args[0].Pos(), To: args[len(args)-1].End()} } @@ -711,8 +710,9 @@ type field struct { typ ast.Expr } -func (p *parser) parseParamDecl(name *ast.Ident) (f field) { - // TODO(rFindley) compare with parser.paramDeclOrNil in the syntax package +func (p *parser) parseParamDecl(name *ast.Ident, typeSetsOK bool) (f field) { + // TODO(rFindley) refactor to be more similar to paramDeclOrNil in the syntax + // package if p.trace { defer un(trace(p, "ParamDeclOrNil")) } @@ -720,10 +720,14 @@ func (p *parser) parseParamDecl(name *ast.Ident) (f field) { ptok := p.tok if name != nil { p.tok = token.IDENT // force token.IDENT case in switch below + } else if typeSetsOK && p.tok == token.TILDE { + // "~" ... + return field{nil, p.embeddedElem(nil)} } switch p.tok { case token.IDENT: + // name if name != nil { f.name = name p.tok = ptok @@ -736,17 +740,32 @@ func (p *parser) parseParamDecl(name *ast.Ident) (f field) { f.typ = p.parseType() case token.LBRACK: - // name[type1, type2, ...] or name []type or name [len]type + // name "[" type1, ..., typeN "]" or name "[" n "]" type f.name, f.typ = p.parseArrayFieldOrTypeInstance(f.name) case token.ELLIPSIS: - // name ...type + // name "..." type f.typ = p.parseDotsType() + return // don't allow ...type "|" ... case token.PERIOD: - // qualified.typename + // name "." ... f.typ = p.parseQualifiedIdent(f.name) f.name = nil + + case token.TILDE: + if typeSetsOK { + f.typ = p.embeddedElem(nil) + return + } + + case token.OR: + if typeSetsOK { + // name "|" typeset + f.typ = p.embeddedElem(f.name) + f.name = nil + return + } } case token.MUL, token.ARROW, token.FUNC, token.LBRACK, token.CHAN, token.MAP, token.STRUCT, token.INTERFACE, token.LPAREN: @@ -754,23 +773,36 @@ func (p *parser) parseParamDecl(name *ast.Ident) (f field) { f.typ = p.parseType() case token.ELLIPSIS: - // ...type + // "..." type // (always accepted) f.typ = p.parseDotsType() + return // don't allow ...type "|" ... default: + // TODO(rfindley): this looks incorrect in the case of type parameter + // lists. p.errorExpected(p.pos, ")") p.advance(exprEnd) } + // [name] type "|" + if typeSetsOK && p.tok == token.OR && f.typ != nil { + f.typ = p.embeddedElem(f.typ) + } + return } -func (p *parser) parseParameterList(name0 *ast.Ident, closing token.Token, parseParamDecl func(*ast.Ident) field, tparams bool) (params []*ast.Field) { +func (p *parser) parseParameterList(name0 *ast.Ident, closing token.Token) (params []*ast.Field) { if p.trace { defer un(trace(p, "ParameterList")) } + // Type parameters are the only parameter list closed by ']'. + tparams := closing == token.RBRACK + // Type set notation is ok in type parameter lists. + typeSetsOK := tparams && p.allowTypeSets() + pos := p.pos if name0 != nil { pos = name0.Pos() @@ -780,7 +812,7 @@ func (p *parser) parseParameterList(name0 *ast.Ident, closing token.Token, parse var named int // number of parameters that have an explicit name and type for name0 != nil || p.tok != closing && p.tok != token.EOF { - par := parseParamDecl(name0) + par := p.parseParamDecl(name0, typeSetsOK) name0 = nil // 1st name was consumed if present if par.name != nil || par.typ != nil { list = append(list, par) @@ -818,11 +850,13 @@ func (p *parser) parseParameterList(name0 *ast.Ident, closing token.Token, parse // some named => all must be named ok := true var typ ast.Expr + missingName := pos for i := len(list) - 1; i >= 0; i-- { if par := &list[i]; par.typ != nil { typ = par.typ if par.name == nil { ok = false + missingName = par.typ.Pos() n := ast.NewIdent("_") n.NamePos = typ.Pos() // correct position par.name = n @@ -832,12 +866,13 @@ func (p *parser) parseParameterList(name0 *ast.Ident, closing token.Token, parse } else { // par.typ == nil && typ == nil => we only have a par.name ok = false + missingName = par.name.Pos() par.typ = &ast.BadExpr{From: par.name.Pos(), To: p.pos} } } if !ok { if tparams { - p.error(pos, "all type parameters must be named") + p.error(missingName, "all type parameters must be named") } else { p.error(pos, "mixed named and unnamed parameters") } @@ -883,11 +918,11 @@ func (p *parser) parseParameters(acceptTParams bool) (tparams, params *ast.Field defer un(trace(p, "Parameters")) } - if p.parseTypeParams() && acceptTParams && p.tok == token.LBRACK { + if p.allowGenerics() && acceptTParams && p.tok == token.LBRACK { opening := p.pos p.next() // [T any](params) syntax - list := p.parseParameterList(nil, token.RBRACK, p.parseParamDecl, true) + list := p.parseParameterList(nil, token.RBRACK) rbrack := p.expect(token.RBRACK) tparams = &ast.FieldList{Opening: opening, List: list, Closing: rbrack} // Type parameter lists must not be empty. @@ -901,7 +936,7 @@ func (p *parser) parseParameters(acceptTParams bool) (tparams, params *ast.Field var fields []*ast.Field if p.tok != token.RPAREN { - fields = p.parseParameterList(nil, token.RPAREN, p.parseParamDecl, false) + fields = p.parseParameterList(nil, token.RPAREN) } rparen := p.expect(token.RPAREN) @@ -956,7 +991,7 @@ func (p *parser) parseMethodSpec() *ast.Field { x := p.parseTypeName(nil) if ident, _ := x.(*ast.Ident); ident != nil { switch { - case p.tok == token.LBRACK && p.parseTypeParams(): + case p.tok == token.LBRACK && p.allowGenerics(): // generic method or embedded instantiated type lbrack := p.pos p.next() @@ -965,7 +1000,7 @@ func (p *parser) parseMethodSpec() *ast.Field { p.exprLev-- if name0, _ := x.(*ast.Ident); name0 != nil && p.tok != token.COMMA && p.tok != token.RBRACK { // generic method m[T any] - list := p.parseParameterList(name0, token.RBRACK, p.parseParamDecl, true) + list := p.parseParameterList(name0, token.RBRACK) rbrack := p.expect(token.RBRACK) tparams := &ast.FieldList{Opening: lbrack, List: list, Closing: rbrack} // TODO(rfindley) refactor to share code with parseFuncType. @@ -1011,7 +1046,7 @@ func (p *parser) parseMethodSpec() *ast.Field { } else { // embedded, possibly instantiated type typ = x - if p.tok == token.LBRACK && p.parseTypeParams() { + if p.tok == token.LBRACK && p.allowGenerics() { // embedded instantiated interface typ = p.parseTypeInstance(typ) } @@ -1024,24 +1059,23 @@ func (p *parser) parseMethodSpec() *ast.Field { return &ast.Field{Doc: doc, Names: idents, Type: typ} } -func (p *parser) embeddedElem(f *ast.Field) *ast.Field { +func (p *parser) embeddedElem(x ast.Expr) ast.Expr { if p.trace { defer un(trace(p, "EmbeddedElem")) } - if f == nil { - f = new(ast.Field) - f.Type = p.embeddedTerm() + if x == nil { + x = p.embeddedTerm() } for p.tok == token.OR { t := new(ast.BinaryExpr) t.OpPos = p.pos t.Op = token.OR p.next() - t.X = f.Type + t.X = x t.Y = p.embeddedTerm() - f.Type = t + x = t } - return f + return x } func (p *parser) embeddedTerm() ast.Expr { @@ -1083,18 +1117,18 @@ parseElements: switch { case p.tok == token.IDENT: f := p.parseMethodSpec() - if f.Names == nil && p.parseTypeParams() { - f = p.embeddedElem(f) + if f.Names == nil && p.allowGenerics() { + f.Type = p.embeddedElem(f.Type) } p.expectSemi() f.Comment = p.lineComment list = append(list, f) - case p.tok == token.TILDE && p.parseTypeParams(): - f := p.embeddedElem(nil) + case p.tok == token.TILDE && p.allowGenerics(): + typ := p.embeddedElem(nil) p.expectSemi() - f.Comment = p.lineComment - list = append(list, f) - case p.tok == token.TYPE && p.parseTypeParams(): + comment := p.lineComment + list = append(list, &ast.Field{Type: typ, Comment: comment}) + case p.tok == token.TYPE && p.allowGenerics(): // TODO(rfindley): remove TypeList syntax and refactor the clauses above. // all types in a type list share the same field name "type" @@ -1106,14 +1140,12 @@ parseElements: list = append(list, &ast.Field{Names: name, Type: typ}) } p.expectSemi() - case p.parseTypeParams(): + case p.allowGenerics(): if t := p.tryIdentOrType(); t != nil { - f := new(ast.Field) - f.Type = t - f = p.embeddedElem(f) + typ := p.embeddedElem(t) p.expectSemi() - f.Comment = p.lineComment - list = append(list, f) + comment := p.lineComment + list = append(list, &ast.Field{Type: typ, Comment: comment}) } else { break parseElements } @@ -1176,7 +1208,7 @@ func (p *parser) parseChanType() *ast.ChanType { } func (p *parser) parseTypeInstance(typ ast.Expr) ast.Expr { - assert(p.parseTypeParams(), "parseTypeInstance while not parsing type params") + assert(p.allowGenerics(), "parseTypeInstance while not parsing type params") if p.trace { defer un(trace(p, "TypeInstance")) } @@ -1212,7 +1244,7 @@ func (p *parser) tryIdentOrType() ast.Expr { switch p.tok { case token.IDENT: typ := p.parseTypeName(nil) - if p.tok == token.LBRACK && p.parseTypeParams() { + if p.tok == token.LBRACK && p.allowGenerics() { typ = p.parseTypeInstance(typ) } return typ @@ -1462,7 +1494,7 @@ func (p *parser) parseIndexOrSliceOrInstance(x ast.Expr) ast.Expr { return &ast.IndexExpr{X: x, Lbrack: lbrack, Index: index[0], Rbrack: rbrack} } - if !p.parseTypeParams() { + if !p.allowGenerics() { p.error(firstComma, "expected ']' or ':', found ','") return &ast.BadExpr{From: args[0].Pos(), To: args[len(args)-1].End()} } @@ -2507,7 +2539,7 @@ func (p *parser) parseValueSpec(doc *ast.CommentGroup, _ token.Pos, keyword toke } func (p *parser) parseGenericType(spec *ast.TypeSpec, openPos token.Pos, name0 *ast.Ident) { - list := p.parseParameterList(name0, token.RBRACK, p.parseParamDecl, true) + list := p.parseParameterList(name0, token.RBRACK) closePos := p.expect(token.RBRACK) spec.TypeParams = &ast.FieldList{Opening: openPos, List: list, Closing: closePos} // Type alias cannot have type parameters. Accept them for robustness but complain. @@ -2535,7 +2567,7 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token p.exprLev++ x := p.parseExpr() p.exprLev-- - if name0, _ := x.(*ast.Ident); p.parseTypeParams() && name0 != nil && p.tok != token.RBRACK { + if name0, _ := x.(*ast.Ident); p.allowGenerics() && name0 != nil && p.tok != token.RBRACK { // generic type [T any]; p.parseGenericType(spec, lbrack, name0) } else { diff --git a/src/go/parser/testdata/typeset.go2 b/src/go/parser/testdata/typeset.go2 new file mode 100644 index 0000000000..aa18e8ccff --- /dev/null +++ b/src/go/parser/testdata/typeset.go2 @@ -0,0 +1,75 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains test cases for typeset-only constraint elements. +// TODO(gri) gofmt once/if gofmt supports this notation. + +package p + +type ( + _[_ t] t + _[_ ~t] t + _[_ t|t] t + _[_ ~t|t] t + _[_ t|~t] t + _[_ ~t|~t] t + + _[_ t, _, _ t|t] t + _[_ t, _, _ ~t|t] t + _[_ t, _, _ t|~t] t + _[_ t, _, _ ~t|~t] t + + _[_ t.t] t + _[_ ~t.t] t + _[_ t.t|t.t] t + _[_ ~t.t|t.t] t + _[_ t.t|~t.t] t + _[_ ~t.t|~t.t] t + + _[_ t, _, _ t.t|t.t] t + _[_ t, _, _ ~t.t|t.t] t + _[_ t, _, _ t.t|~t.t] t + _[_ t, _, _ ~t.t|~t.t] t + + _[_ struct{}] t + _[_ ~struct{}] t + + _[_ struct{}|t] t + _[_ ~struct{}|t] t + _[_ struct{}|~t] t + _[_ ~struct{}|~t] t + + _[_ t|struct{}] t + _[_ ~t|struct{}] t + _[_ t|~struct{}] t + _[_ ~t|~struct{}] t +) + +// Single-expression type parameter lists and those that don't start +// with a (type parameter) name are considered array sizes. +// The term must be a valid expression (it could be a type - and then +// a type-checker will complain - but we don't allow ~ in the expr). +// TODO(rfindley): Improve error recover here. In these cases go/parser error +// recovery is worse than cmd/compile/internal/syntax, and unnecessary type +// declarations had to be inserted to force synchronization. +type _[t] t +type _[~ /* ERROR "expected operand" */ t] t +type /* ERROR "expected ']'" */ Sync int // placeholder to synchronize the parser +type _[t|t] t +type _[~ /* ERROR "expected operand" */ t|t] t +type /* ERROR "expected ']'" */ Sync int // placeholder to synchronize the parser +type _[t| ~ /* ERROR "expected operand" */ t] t +type /* ERROR "expected ']'" */ Sync int // placeholder to synchronize the parser +type _[~ /* ERROR "expected operand" */ t|~t] t +type /* ERROR "expected ']'" */ Sync int // placeholder to synchronize the parser + +type _[_ t, t /* ERROR "type parameters must be named" */ ] t +type _[_ ~t, t /* ERROR "type parameters must be named" */ ] t +type _[_ t, ~ /* ERROR "type parameters must be named" */ t] t +type _[_ ~t, ~ /* ERROR "type parameters must be named" */ t] t + +type _[_ t|t, t /* ERROR "type parameters must be named" */ |t] t +type _[_ ~t|t, t /* ERROR "type parameters must be named" */ |t] t +type _[_ t|t, ~ /* ERROR "type parameters must be named" */ t|t] t +type _[_ ~t|t, ~ /* ERROR "type parameters must be named" */ t|t] t diff --git a/src/go/printer/testdata/generics.golden b/src/go/printer/testdata/generics.golden index cc7fbbe1d8..31ab7716dd 100644 --- a/src/go/printer/testdata/generics.golden +++ b/src/go/printer/testdata/generics.golden @@ -36,8 +36,13 @@ func _() { } // properly format one-line type lists +// TODO(rfindley): remove support for type lists type _ interface{ type a } type _ interface { type a, b, c } + +// type constraint literals with elided interfaces +func _[P ~int, Q int | string]() {} +func _[P struct{ f int }, Q *P]() {} diff --git a/src/go/printer/testdata/generics.input b/src/go/printer/testdata/generics.input index f4571ad336..11431c5a0a 100644 --- a/src/go/printer/testdata/generics.input +++ b/src/go/printer/testdata/generics.input @@ -33,6 +33,11 @@ func _() { } // properly format one-line type lists +// TODO(rfindley): remove support for type lists type _ interface { type a } type _ interface { type a,b,c } + +// type constraint literals with elided interfaces +func _[P ~int, Q int | string]() {} +func _[P struct{f int}, Q *P]() {} -- 2.50.0