From 920b9ab57dc6be573e8da705f13cf17ebab65342 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 20 Apr 2022 22:19:49 -0700 Subject: [PATCH] cmd/compile/internal/syntax: accept all valid type parameter lists Type parameter lists starting with the form [name *T|...] or [name (X)|...] may look like an array length expression [x]. Only after parsing the entire initial expression and checking whether the expression contains type elements or is followed by a comma can we make the final decision. This change simplifies the existing parsing strategy: instead of trying to make an upfront decision with limited information (which is insufficient), the parser now parses the start of a type parameter list or array length specification as expression. In a second step, if the expression can be split into a name followed by a type element, or a name followed by an ordinary expression which is succeeded by a comma, we assume a type parameter list (because it can't be an array length). In all other cases we assume an array length specification. Fixes #49482. Change-Id: I269b6291999bf60dc697d33d24a5635f01e065b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/402256 Reviewed-by: Benny Siegert Reviewed-by: Ian Lance Taylor Reviewed-by: Ian Lance Taylor --- src/cmd/compile/internal/syntax/parser.go | 125 +++++++++++------- src/cmd/compile/internal/syntax/printer.go | 21 +-- .../compile/internal/syntax/printer_test.go | 57 +++++--- .../internal/syntax/testdata/tparams.go | 22 +++ .../types2/testdata/fixedbugs/issue49482.go | 21 +-- 5 files changed, 158 insertions(+), 88 deletions(-) diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go index a89dcfae52..aaeb2a23c6 100644 --- a/src/cmd/compile/internal/syntax/parser.go +++ b/src/cmd/compile/internal/syntax/parser.go @@ -599,10 +599,12 @@ func (p *parser) typeDecl(group *Group) Decl { // with a "[" as in: P []E. In that case, simply parsing // an expression would lead to an error: P[] is invalid. // But since index or slice expressions are never constant - // and thus invalid array length expressions, if we see a - // "[" following a name it must be the start of an array - // or slice constraint. Only if we don't see a "[" do we - // need to parse a full expression. + // and thus invalid array length expressions, if the name + // is followed by "[" it must be the start of an array or + // slice constraint. Only if we don't see a "[" do we + // need to parse a full expression. Notably, name <- x + // is not a concern because name <- x is a statement and + // not an expression. var x Expr = p.name() if p.tok != _Lbrack { // To parse the expression starting with name, expand @@ -612,53 +614,22 @@ func (p *parser) typeDecl(group *Group) Decl { x = p.binaryExpr(p.pexpr(x, false), 0) p.xnest-- } - - // analyze the cases - var pname *Name // pname != nil means pname is the type parameter name - var ptype Expr // ptype != nil means ptype is the type parameter type; pname != nil in this case - switch t := x.(type) { - case *Name: - // Unless we see a "]", we are at the start of a type parameter list. - if p.tok != _Rbrack { - // d.Name "[" name ... - pname = t - // no ptype - } - case *Operation: - // If we have an expression of the form name*T, and T is a (possibly - // parenthesized) type literal or the next token is a comma, we are - // at the start of a type parameter list. - if name, _ := t.X.(*Name); name != nil { - if t.Op == Mul && (isTypeLit(t.Y) || p.tok == _Comma) { - // d.Name "[" name "*" t.Y - // d.Name "[" name "*" t.Y "," - t.X, t.Y = t.Y, nil // convert t into unary *t.Y - pname = name - ptype = t - } - } - case *CallExpr: - // If we have an expression of the form name(T), and T is a (possibly - // parenthesized) type literal or the next token is a comma, we are - // at the start of a type parameter list. - if name, _ := t.Fun.(*Name); name != nil { - if len(t.ArgList) == 1 && !t.HasDots && (isTypeLit(t.ArgList[0]) || p.tok == _Comma) { - // d.Name "[" name "(" t.ArgList[0] ")" - // d.Name "[" name "(" t.ArgList[0] ")" "," - pname = name - ptype = t.ArgList[0] - } - } - } - - if pname != nil { + // Analyze expression x. If we can split x into a type parameter + // name, possibly followed by a type parameter type, we consider + // this the start of a type parameter list, with some caveats: + // a single name followed by "]" tilts the decision towards an + // array declaration; a type parameter type that could also be + // an ordinary expression but which is followed by a comma tilts + // the decision towards a type parameter list. + if pname, ptype := extractName(x, p.tok == _Comma); pname != nil && (ptype != nil || p.tok != _Rbrack) { // d.Name "[" pname ... // d.Name "[" pname ptype ... // d.Name "[" pname ptype "," ... - d.TParamList = p.paramList(pname, ptype, _Rbrack, true) + d.TParamList = p.paramList(pname, ptype, _Rbrack, true) // ptype may be nil d.Alias = p.gotAssign() d.Type = p.typeOrNil() } else { + // d.Name "[" pname "]" ... // d.Name "[" x ... d.Type = p.arrayType(pos, x) } @@ -684,17 +655,69 @@ func (p *parser) typeDecl(group *Group) Decl { return d } -// isTypeLit reports whether x is a (possibly parenthesized) type literal. -func isTypeLit(x Expr) bool { +// extractName splits the expression x into (name, expr) if syntactically +// x can be written as name expr. The split only happens if expr is a type +// element (per the isTypeElem predicate) or if force is set. +// If x is just a name, the result is (name, nil). If the split succeeds, +// the result is (name, expr). Otherwise the result is (nil, x). +// Examples: +// +// x force name expr +// ------------------------------------ +// P*[]int T/F P *[]int +// P*E T P *E +// P*E F nil P*E +// P([]int) T/F P []int +// P(E) T P E +// P(E) F nil P(E) +// P*E|F|~G T/F P *E|F|~G +// P*E|F|G T P *E|F|G +// P*E|F|G F nil P*E|F|G +func extractName(x Expr, force bool) (*Name, Expr) { + switch x := x.(type) { + case *Name: + return x, nil + case *Operation: + if x.Y == nil { + break // unary expr + } + switch x.Op { + case Mul: + if name, _ := x.X.(*Name); name != nil && (isTypeElem(x.Y) || force) { + // x = name *x.Y + op := *x + op.X, op.Y = op.Y, nil // change op into unary *op.Y + return name, &op + } + case Or: + if name, lhs := extractName(x.X, isTypeElem(x.Y) || force); name != nil && lhs != nil { // note: lhs should never be nil + // x = name lhs|x.Y + op := *x + op.X = lhs + return name, &op + } + } + case *CallExpr: + if name, _ := x.Fun.(*Name); name != nil { + if len(x.ArgList) == 1 && !x.HasDots && (isTypeElem(x.ArgList[0]) || force) { + // x = name "(" x.ArgList[0] ")" + return name, x.ArgList[0] + } + } + } + return nil, x +} + +// isTypeElem reports whether x is a (possibly parenthesized) type element expression. +// The result is false if x could be a type element OR an ordinary (value) expression. +func isTypeElem(x Expr) bool { switch x := x.(type) { case *ArrayType, *StructType, *FuncType, *InterfaceType, *SliceType, *MapType, *ChanType: return true case *Operation: - // *T may be a pointer dereferenciation. - // Only consider *T as type literal if T is a type literal. - return x.Op == Mul && x.Y == nil && isTypeLit(x.X) + return isTypeElem(x.X) || (x.Y != nil && isTypeElem(x.Y)) || x.Op == Tilde case *ParenExpr: - return isTypeLit(x.X) + return isTypeElem(x.X) } return false } diff --git a/src/cmd/compile/internal/syntax/printer.go b/src/cmd/compile/internal/syntax/printer.go index 0385227c7c..ff3fd9bf47 100644 --- a/src/cmd/compile/internal/syntax/printer.go +++ b/src/cmd/compile/internal/syntax/printer.go @@ -666,7 +666,7 @@ func (p *printer) printRawNode(n Node) { } p.print(n.Name) if n.TParamList != nil { - p.printParameterList(n.TParamList, true) + p.printParameterList(n.TParamList, _Type) } p.print(blank) if n.Alias { @@ -698,7 +698,7 @@ func (p *printer) printRawNode(n Node) { } p.print(n.Name) if n.TParamList != nil { - p.printParameterList(n.TParamList, true) + p.printParameterList(n.TParamList, _Func) } p.printSignature(n.Type) if n.Body != nil { @@ -883,20 +883,23 @@ func (p *printer) printDeclList(list []Decl) { } func (p *printer) printSignature(sig *FuncType) { - p.printParameterList(sig.ParamList, false) + p.printParameterList(sig.ParamList, 0) if list := sig.ResultList; list != nil { p.print(blank) if len(list) == 1 && list[0].Name == nil { p.printNode(list[0].Type) } else { - p.printParameterList(list, false) + p.printParameterList(list, 0) } } } -func (p *printer) printParameterList(list []*Field, types bool) { +// If tok != 0 print a type parameter list: tok == _Type means +// a type parameter list for a type, tok == _Func means a type +// parameter list for a func. +func (p *printer) printParameterList(list []*Field, tok token) { open, close := _Lparen, _Rparen - if types { + if tok != 0 { open, close = _Lbrack, _Rbrack } p.print(open) @@ -916,10 +919,10 @@ func (p *printer) printParameterList(list []*Field, types bool) { } p.printNode(unparen(f.Type)) // no need for (extra) parentheses around parameter types } - // A type parameter list [P *T] where T is not a type literal requires a comma as in [P *T,] + // A type parameter list [P *T] where T is not a type element requires a comma as in [P *T,] // so that it's not parsed as [P*T]. - if types && len(list) == 1 { - if t, _ := list[0].Type.(*Operation); t != nil && t.Op == Mul && t.Y == nil && !isTypeLit(t.X) { + if tok == _Type && len(list) == 1 { + if t, _ := list[0].Type.(*Operation); t != nil && !isTypeElem(t) { p.print(_Comma) } } diff --git a/src/cmd/compile/internal/syntax/printer_test.go b/src/cmd/compile/internal/syntax/printer_test.go index 3eca2316a7..25155e5cc6 100644 --- a/src/cmd/compile/internal/syntax/printer_test.go +++ b/src/cmd/compile/internal/syntax/printer_test.go @@ -57,11 +57,12 @@ var stringTests = [][2]string{ dup("package p"), dup("package p; type _ int; type T1 = struct{}; type ( _ *struct{}; T2 = float32 )"), - // generic type declarations + // generic type declarations (given type separated with blank from LHS) dup("package p; type _[T any] struct{}"), dup("package p; type _[A, B, C interface{m()}] struct{}"), dup("package p; type _[T any, A, B, C interface{m()}, X, Y, Z interface{~int}] struct{}"), + dup("package p; type _[P *struct{}] struct{}"), dup("package p; type _[P *T,] struct{}"), dup("package p; type _[P *T, _ any] struct{}"), {"package p; type _[P (*T),] struct{}", "package p; type _[P *T,] struct{}"}, @@ -69,36 +70,56 @@ var stringTests = [][2]string{ {"package p; type _[P (T),] struct{}", "package p; type _[P T] struct{}"}, {"package p; type _[P (T), _ any] struct{}", "package p; type _[P T, _ any] struct{}"}, - dup("package p; type _[P *struct{}] struct{}"), {"package p; type _[P (*struct{})] struct{}", "package p; type _[P *struct{}] struct{}"}, {"package p; type _[P ([]int)] struct{}", "package p; type _[P []int] struct{}"}, - - dup("package p; type _ [P(T)]struct{}"), - dup("package p; type _ [P((T))]struct{}"), - dup("package p; type _ [P * *T]struct{}"), - dup("package p; type _ [P * T]struct{}"), - dup("package p; type _ [P(*T)]struct{}"), - dup("package p; type _ [P(**T)]struct{}"), - dup("package p; type _ [P * T - T]struct{}"), - - // array type declarations - dup("package p; type _ [P * T]struct{}"), - dup("package p; type _ [P * T - T]struct{}"), + {"package p; type _[P ([]int) | int] struct{}", "package p; type _[P []int | int] struct{}"}, + + // a type literal in an |-expression indicates a type parameter list (blank after type parameter list and type) + dup("package p; type _[P *[]int] struct{}"), + dup("package p; type _[P *T | T, Q T] struct{}"), + dup("package p; type _[P *[]T | T] struct{}"), + dup("package p; type _[P *T | T | T | T | ~T] struct{}"), + dup("package p; type _[P *T | T | T | ~T | T] struct{}"), + dup("package p; type _[P *T | T | struct{} | T] struct{}"), + dup("package p; type _[P <-chan int] struct{}"), + dup("package p; type _[P *T | struct{} | T] struct{}"), + + // a trailing comma always indicates a type parameter list (blank after type parameter list and type) + dup("package p; type _[P *T,] struct{}"), + dup("package p; type _[P *T | T,] struct{}"), + dup("package p; type _[P *T | <-T | T,] struct{}"), + + // slice/array type declarations (no blank between array length and element type) + dup("package p; type _ []byte"), + dup("package p; type _ [n]byte"), + dup("package p; type _ [P(T)]byte"), + dup("package p; type _ [P((T))]byte"), + dup("package p; type _ [P * *T]byte"), + dup("package p; type _ [P * T]byte"), + dup("package p; type _ [P(*T)]byte"), + dup("package p; type _ [P(**T)]byte"), + dup("package p; type _ [P * T - T]byte"), + dup("package p; type _ [P * T - T]byte"), + dup("package p; type _ [P * T | T]byte"), + dup("package p; type _ [P * T | <-T | T]byte"), // generic function declarations dup("package p; func _[T any]()"), dup("package p; func _[A, B, C interface{m()}]()"), dup("package p; func _[T any, A, B, C interface{m()}, X, Y, Z interface{~int}]()"), + // generic functions with elided interfaces in type constraints + dup("package p; func _[P *T]() {}"), + dup("package p; func _[P *T | T | T | T | ~T]() {}"), + dup("package p; func _[P *T | T | struct{} | T]() {}"), + dup("package p; func _[P ~int, Q int | string]() {}"), + dup("package p; func _[P struct{f int}, Q *P]() {}"), + // methods with generic receiver types dup("package p; func (R[T]) _()"), dup("package p; func (*R[A, B, C]) _()"), dup("package p; func (_ *R[A, B, C]) _()"), - // type constraint literals with elided interfaces - dup("package p; func _[P ~int, Q int | string]() {}"), - dup("package p; func _[P struct{f int}, Q *P]() {}"), - // channels dup("package p; type _ chan chan int"), dup("package p; type _ chan (<-chan int)"), diff --git a/src/cmd/compile/internal/syntax/testdata/tparams.go b/src/cmd/compile/internal/syntax/testdata/tparams.go index a9bd72cf2d..671833f931 100644 --- a/src/cmd/compile/internal/syntax/testdata/tparams.go +++ b/src/cmd/compile/internal/syntax/testdata/tparams.go @@ -22,3 +22,25 @@ func f[a, b /* ERROR missing type constraint */ ]() func f[a t, b t, c /* ERROR missing type constraint */ ]() func f[a b, /* ERROR expecting ] */ 0] () + +// issue #49482 +type ( + t[a *[]int] struct{} + t[a *t,] struct{} + t[a *t|[]int] struct{} + t[a *t|t,] struct{} + t[a *t|~t,] struct{} + t[a *struct{}|t] struct{} + t[a *t|struct{}] struct{} + t[a *struct{}|~t] struct{} +) + +// issue #51488 +type ( + t[a *t|t,] struct{} + t[a *t|t, b t] struct{} + t[a *t|t] struct{} + t[a *[]t|t] struct{} + t[a ([]t)] struct{} + t[a ([]t)|t] struct{} +) diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49482.go b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49482.go index 503d9946b4..d5c52dc288 100644 --- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49482.go +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue49482.go @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This file is tested when running "go test -run Manual" -// without source arguments. Use for one-off debugging. - package p // The following is OK, per the special handling for type literals discussed in issue #49482. @@ -14,12 +11,16 @@ type _[P (*int),] int const P = 2 // declare P to avoid noisy 'undeclared name' errors below. -// The following parse as invalid array types. -type _[P *int /* ERROR "int \(type\) is not an expression" */ ] int -type _[P /* ERROR non-function P */ (*int)] int +// The following parse as invalid array types due to parsing ambiguitiues. +type _ [P *int /* ERROR "int \(type\) is not an expression" */ ]int +type _ [P /* ERROR non-function P */ (*int)]int -// The following should be parsed as a generic type, but is instead parsed as an array type. -type _[P *struct /* ERROR "not an expression" */ {}| int /* ERROR "not an expression" */ ] struct{} +// Adding a trailing comma or an enclosing interface resolves the ambiguity. +type _[P *int,] int +type _[P (*int),] int +type _[P interface{*int}] int +type _[P interface{(*int)}] int -// The following fails to parse, due to the '~' -type _[P *struct /* ERROR "not an expression" */ {}|~int /* ERROR "not an expression" */ ] struct{} +// The following parse correctly as valid generic types. +type _[P *struct{} | int] struct{} +type _[P *struct{} | ~int] struct{} -- 2.50.0