]> Cypherpunks repositories - gostls13.git/commitdiff
go/parser: accept all valid type parameter lists
authorRobert Griesemer <gri@golang.org>
Tue, 3 May 2022 17:42:22 +0000 (10:42 -0700)
committerRobert Griesemer <gri@golang.org>
Wed, 4 May 2022 20:20:27 +0000 (20:20 +0000)
This is a port of CL 402256 from the syntax package to go/parser
with adjustments because of the different AST structure, and
excluding any necessary go/printer changes (separate CL).

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 #52559.

Change-Id: I11ab6e62b073b78b2331bb6063cf74d2a9eaa236
Reviewed-on: https://go-review.googlesource.com/c/go/+/403937
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
src/go/parser/parser.go
src/go/parser/testdata/issue49482.go2
src/go/parser/testdata/tparams.go2 [new file with mode: 0644]
src/go/printer/testdata/generics.golden
src/go/types/testdata/fixedbugs/issue49482.go

index 3c9be3162611a9366ac1fe051e0a37d6c682d7f1..18041ff808d1e61a21af5c2d161c820924223379 100644 (file)
@@ -785,9 +785,9 @@ func (p *parser) parseParamDecl(name *ast.Ident, typeSetsOK bool) (f field) {
                return // don't allow ...type "|" ...
 
        default:
-               // TODO(rfindley): this looks incorrect in the case of type parameter
-               // lists.
-               p.errorExpected(p.pos, ")")
+               // TODO(rfindley): this is incorrect in the case of type parameter lists
+               //                 (should be "']'" in that case)
+               p.errorExpected(p.pos, "')'")
                p.advance(exprEnd)
        }
 
@@ -2592,10 +2592,12 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
                defer un(trace(p, "TypeSpec"))
        }
 
-       ident := p.parseIdent()
-       spec := &ast.TypeSpec{Doc: doc, Name: ident}
+       name := p.parseIdent()
+       spec := &ast.TypeSpec{Doc: doc, Name: name}
 
        if p.tok == token.LBRACK && p.allowGenerics() {
+               // spec.Name "[" ...
+               // array/slice type or type parameter list
                lbrack := p.pos
                p.next()
                if p.tok == token.IDENT {
@@ -2608,14 +2610,12 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
                        // 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.
-
-                       // Index or slice expressions are never constant and thus invalid
-                       // array length expressions. Thus, if we see a "[" following name
-                       // we can safely assume that "[" name starts a type parameter list.
+                       // 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 ast.Expr = p.parseIdent()
                        if p.tok != token.LBRACK {
                                // To parse the expression starting with name, expand
@@ -2626,58 +2626,21 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
                                x = p.parseBinaryExpr(lhs, token.LowestPrec+1, false)
                                p.exprLev--
                        }
-
-                       // analyze the cases
-                       var pname *ast.Ident // pname != nil means pname is the type parameter name
-                       var ptype ast.Expr   // ptype != nil means ptype is the type parameter type; pname != nil in this case
-
-                       switch t := x.(type) {
-                       case *ast.Ident:
-                               // Unless we see a "]", we are at the start of a type parameter list.
-                               if p.tok != token.RBRACK {
-                                       // d.Name "[" name ...
-                                       pname = t
-                                       // no ptype
-                               }
-                       case *ast.BinaryExpr:
-                               // 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.(*ast.Ident); name != nil {
-                                       if t.Op == token.MUL && (isTypeLit(t.Y) || p.tok == token.COMMA) {
-                                               // d.Name "[" name "*" t.Y
-                                               // d.Name "[" name "*" t.Y ","
-                                               // convert t into unary *t.Y
-                                               pname = name
-                                               ptype = &ast.StarExpr{Star: t.OpPos, X: t.Y}
-                                       }
-                               }
-                               if pname == nil {
-                                       // A normal binary expression. Since we passed check=false, we must
-                                       // now check its operands.
-                                       p.checkBinaryExpr(t)
-                               }
-                       case *ast.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.(*ast.Ident); name != nil {
-                                       if len(t.Args) == 1 && !t.Ellipsis.IsValid() && (isTypeLit(t.Args[0]) || p.tok == token.COMMA) {
-                                               // d.Name "[" name "(" t.ArgList[0] ")"
-                                               // d.Name "[" name "(" t.ArgList[0] ")" ","
-                                               pname = name
-                                               ptype = t.Args[0]
-                                       }
-                               }
-                       }
-
-                       if pname != nil {
-                               // d.Name "[" pname ...
-                               // d.Name "[" pname ptype ...
-                               // d.Name "[" pname ptype "," ...
-                               p.parseGenericType(spec, lbrack, pname, ptype)
+                       // 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 == token.COMMA); pname != nil && (ptype != nil || p.tok != token.RBRACK) {
+                               // spec.Name "[" pname ...
+                               // spec.Name "[" pname ptype ...
+                               // spec.Name "[" pname ptype "," ...
+                               p.parseGenericType(spec, lbrack, pname, ptype) // ptype may be nil
                        } else {
-                               // d.Name "[" x ...
+                               // spec.Name "[" pname "]" ...
+                               // spec.Name "[" x ...
                                spec.Type = p.parseArrayType(lbrack, x)
                        }
                } else {
@@ -2700,17 +2663,66 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
        return spec
 }
 
-// isTypeLit reports whether x is a (possibly parenthesized) type literal.
-func isTypeLit(x ast.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 ast.Expr, force bool) (*ast.Ident, ast.Expr) {
+       switch x := x.(type) {
+       case *ast.Ident:
+               return x, nil
+       case *ast.BinaryExpr:
+               switch x.Op {
+               case token.MUL:
+                       if name, _ := x.X.(*ast.Ident); name != nil && (force || isTypeElem(x.Y)) {
+                               // x = name *x.Y
+                               return name, &ast.StarExpr{Star: x.OpPos, X: x.Y}
+                       }
+               case token.OR:
+                       if name, lhs := extractName(x.X, force || isTypeElem(x.Y)); name != nil && lhs != nil {
+                               // x = name lhs|x.Y
+                               op := *x
+                               op.X = lhs
+                               return name, &op
+                       }
+               }
+       case *ast.CallExpr:
+               if name, _ := x.Fun.(*ast.Ident); name != nil {
+                       if len(x.Args) == 1 && x.Ellipsis == token.NoPos && (force || isTypeElem(x.Args[0])) {
+                               // x = name "(" x.ArgList[0] ")"
+                               return name, x.Args[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 ast.Expr) bool {
        switch x := x.(type) {
        case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType:
                return true
-       case *ast.StarExpr:
-               // *T may be a pointer dereferenciation.
-               // Only consider *T as type literal if T is a type literal.
-               return isTypeLit(x.X)
+       case *ast.BinaryExpr:
+               return isTypeElem(x.X) || isTypeElem(x.Y)
+       case *ast.UnaryExpr:
+               return x.Op == token.TILDE
        case *ast.ParenExpr:
-               return isTypeLit(x.X)
+               return isTypeElem(x.X)
        }
        return false
 }
index 50de65118ebd2b1fab91485039893e09f70d0da2..d8385bed4f524030ad3be8be076672530e5c79ee 100644 (file)
@@ -29,7 +29,6 @@ type (
         _ [P*T-T, /* ERROR "unexpected comma" */ ]struct{}
         _ [10, /* ERROR "unexpected comma" */ ]struct{}
 
-        // These should be parsed as generic type declarations.
-        _[P *struct /* ERROR "expected expression" */ {}|int] struct{}
-        _[P *struct /* ERROR "expected expression" */ {}|int|string] struct{}
+        _[P *struct{}|int] struct{}
+        _[P *struct{}|int|string] struct{}
 )
diff --git a/src/go/parser/testdata/tparams.go2 b/src/go/parser/testdata/tparams.go2
new file mode 100644 (file)
index 0000000..28fd132
--- /dev/null
@@ -0,0 +1,47 @@
+// Copyright 2020 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.
+
+package p
+
+type _[a /* ERROR "all type parameters must be named" */, b] struct{}
+type _[a t, b t, c /* ERROR "all type parameters must be named" */ ] struct{}
+type _ struct {
+       t [n]byte
+       t[a]
+       t[a, b]
+}
+type _ interface {
+       t[a]
+       m[ /* ERROR "method must have no type parameters" */ _ _, /* ERROR mixed */ _]()
+       t[a, b]
+}
+
+func _[] /* ERROR "empty type parameter list" */ ()
+func _[a /* ERROR "all type parameters must be named" */, b ]()
+func _[a t, b t, c /* ERROR "all type parameters must be named" */ ]()
+
+// TODO(rfindley) incorrect error message (see existing TODO in parser)
+func f[a b, 0 /* ERROR "expected '\)', found 0" */ ] ()
+
+// issue #49482
+type (
+       _[a *[]int] struct{}
+       _[a *t,] struct{}
+       _[a *t|[]int] struct{}
+       _[a *t|t,] struct{}
+       _[a *t|~t,] struct{}
+       _[a *struct{}|t] struct{}
+       _[a *t|struct{}] struct{}
+       _[a *struct{}|~t] struct{}
+)
+
+// issue #51488
+type (
+       _[a *t|t,] struct{}
+       _[a *t|t, b t] struct{}
+       _[a *t|t] struct{}
+       _[a *[]t|t] struct{}
+       _[a ([]t)] struct{}
+       _[a ([]t)|t] struct{}
+)
index c3a7df83725205afc4cec95cd73f57d477fbf981..f19341680ce85e35fa8d0c4b4b4b59828749ea3d 100644 (file)
@@ -48,7 +48,7 @@ type _[P T] struct{}
 type _[P T, _ any] struct{}
 
 type _[P *struct{}] struct{}
-type _[P *struct{}] struct{}
+type _ [P(*struct{})]struct{}
 type _[P []int] struct{}
 
 // array type declarations
index f103d3b952f9739d62f9c21607af564370dcbcbb..d5c52dc288466533f03d72b824a9dfcadcf4f6d5 100644 (file)
@@ -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 "expected 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 "expected 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{}