go/parser, go/types, syntax, types2: report invalid uses of ... by parsers

Check correct use of ...'s in parameter lists in parsers.
This allows the type checkers to assume correct ASTs with
respect to ... use.

Adjust some error messages: if a ... is used in a result
parameter list, the error is now more accurate.

Eliminate a now unused error code.

Change-Id: I66058e114e84805e24c59e570604b607ef5ff1fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/631135
Reviewed-by: Robert Griesemer <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Bypass: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>

Author: griesemer

src/cmd/compile/internal/syntax/parser.go

@@ -650,7 +650,7 @@ func (p *parser) typeDecl(group *Group) Decl {
 				// d.Name "[" pname ...
 				// d.Name "[" pname ptype ...
 				// d.Name "[" pname ptype "," ...
-				d.TParamList = p.paramList(pname, ptype, _Rbrack, true) // ptype may be nil
+				d.TParamList = p.paramList(pname, ptype, _Rbrack, true, false) // ptype may be nil
 				d.Alias = p.gotAssign()
 				d.Type = p.typeOrNil()
 			} else {
@@ -800,7 +800,7 @@ func (p *parser) funcDeclOrNil() *FuncDecl {
 	var context string
 	if p.got(_Lparen) {
 		context = "method"
-		rcvr := p.paramList(nil, nil, _Rparen, false)
+		rcvr := p.paramList(nil, nil, _Rparen, false, false)
 		switch len(rcvr) {
 		case 0:
 			p.error("method has no receiver")
@@ -1469,12 +1469,12 @@ func (p *parser) funcType(context string) ([]*Field, *FuncType) {
 			p.syntaxError("empty type parameter list")
 			p.next()
 		} else {
-			tparamList = p.paramList(nil, nil, _Rbrack, true)
+			tparamList = p.paramList(nil, nil, _Rbrack, true, false)
 		}
 	}
 
 	p.want(_Lparen)
-	typ.ParamList = p.paramList(nil, nil, _Rparen, false)
+	typ.ParamList = p.paramList(nil, nil, _Rparen, false, true)
 	typ.ResultList = p.funcResult()
 
 	return tparamList, typ
@@ -1582,7 +1582,7 @@ func (p *parser) funcResult() []*Field {
 	}
 
 	if p.got(_Lparen) {
-		return p.paramList(nil, nil, _Rparen, false)
+		return p.paramList(nil, nil, _Rparen, false, false)
 	}
 
 	pos := p.pos()
@@ -1793,7 +1793,7 @@ func (p *parser) methodDecl() *Field {
 
 		// A type argument list looks like a parameter list with only
 		// types. Parse a parameter list and decide afterwards.
-		list := p.paramList(nil, nil, _Rbrack, false)
+		list := p.paramList(nil, nil, _Rbrack, false, false)
 		if len(list) == 0 {
 			// The type parameter list is not [] but we got nothing
 			// due to other errors (reported by paramList). Treat
@@ -1962,10 +1962,11 @@ func (p *parser) paramDeclOrNil(name *Name, follow token) *Field {
 		p.next()
 		t.Elem = p.typeOrNil()
 		if t.Elem == nil {
-			t.Elem = p.badExpr()
+			f.Type = p.badExpr()
 			p.syntaxError("... is missing type")
+		} else {
+			f.Type = t
 		}
-		f.Type = t
 		return f
 	}
 
@@ -1995,7 +1996,7 @@ func (p *parser) paramDeclOrNil(name *Name, follow token) *Field {
 // If name != nil, it is the first name after "(" or "[".
 // If typ != nil, name must be != nil, and (name, typ) is the first field in the list.
 // In the result list, either all fields have a name, or no field has a name.
-func (p *parser) paramList(name *Name, typ Expr, close token, requireNames bool) (list []*Field) {
+func (p *parser) paramList(name *Name, typ Expr, close token, requireNames, dddok bool) (list []*Field) {
 	if trace {
 		defer p.trace("paramList")()
 	}
@@ -2109,6 +2110,23 @@ func (p *parser) paramList(name *Name, typ Expr, close token, requireNames bool)
 		}
 	}
 
+	// check use of ...
+	first := true // only report first occurrence
+	for i, f := range list {
+		if t, _ := f.Type.(*DotsType); t != nil && (!dddok || i+1 < len(list)) {
+			if first {
+				first = false
+				if dddok {
+					p.errorAt(t.pos, "can only use ... with final parameter")
+				} else {
+					p.errorAt(t.pos, "invalid use of ...")
+				}
+			}
+			// use T instead of invalid ...T
+			f.Type = t.Elem
+		}
+	}
+
 	return
 }
 

src/cmd/compile/internal/types2/expr.go

@@ -1016,9 +1016,8 @@ func (check *Checker) exprInternal(T *target, x *operand, e syntax.Expr, hint Ty
 		check.ident(x, e, nil, false)
 
 	case *syntax.DotsType:
-		// dots are handled explicitly where they are legal
-		// (array composite literals and parameter lists)
-		check.error(e, BadDotDotDotSyntax, "invalid use of '...'")
+		// dots are handled explicitly where they are valid
+		check.error(e, InvalidSyntaxTree, "invalid use of ...")
 		goto Error
 
 	case *syntax.BasicLit:

src/cmd/compile/internal/types2/signature.go

@@ -344,7 +344,7 @@ func (check *Checker) collectParams(list []*syntax.Field, variadicOk bool) (name
 				if variadicOk && i == len(list)-1 {
 					variadic = true
 				} else {
-					check.softErrorf(t, MisplacedDotDotDot, "can only use ... with final parameter in list")
+					check.error(t, InvalidSyntaxTree, "invalid use of ...")
 					// ignore ... and continue
 				}
 			}

src/cmd/compile/internal/types2/typexpr.go

@@ -321,10 +321,8 @@ func (check *Checker) typInternal(e0 syntax.Expr, def *TypeName) (T Type) {
 		return typ
 
 	case *syntax.DotsType:
-		// dots are handled explicitly where they are legal
-		// (array composite literals and parameter lists)
-		check.error(e, InvalidDotDotDot, "invalid use of '...'")
-		check.use(e.Elem)
+		// dots are handled explicitly where they are valid
+		check.error(e, InvalidSyntaxTree, "invalid use of ...")
 
 	case *syntax.StructType:
 		typ := new(Struct)

src/go/parser/parser.go

@@ -872,7 +872,7 @@ func (p *parser) parseParamDecl(name *ast.Ident, typeSetsOK bool) (f field) {
 	return
 }
 
-func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing token.Token) (params []*ast.Field) {
+func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing token.Token, dddok bool) (params []*ast.Field) {
 	if p.trace {
 		defer un(trace(p, "ParameterList"))
 	}
@@ -1006,6 +1006,26 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 		}
 	}
 
+	// check use of ...
+	first := true // only report first occurrence
+	for i, _ := range list {
+		f := &list[i]
+		if t, _ := f.typ.(*ast.Ellipsis); t != nil && (!dddok || i+1 < len(list)) {
+			if first {
+				first = false
+				if dddok {
+					p.error(t.Ellipsis, "can only use ... with final parameter")
+				} else {
+					p.error(t.Ellipsis, "invalid use of ...")
+				}
+			}
+			// use T instead of invalid ...T
+			// TODO(gri) would like to use `f.typ = t.Elt` but that causes problems
+			//           with the resolver in cases of reuse of the same identifier
+			f.typ = &ast.BadExpr{From: t.Pos(), To: t.End()}
+		}
+	}
+
 	// Convert list to []*ast.Field.
 	// If list contains types only, each type gets its own ast.Field.
 	if named == 0 {
@@ -1050,7 +1070,7 @@ func (p *parser) parseTypeParameters() *ast.FieldList {
 	lbrack := p.expect(token.LBRACK)
 	var list []*ast.Field
 	if p.tok != token.RBRACK {
-		list = p.parseParameterList(nil, nil, token.RBRACK)
+		list = p.parseParameterList(nil, nil, token.RBRACK, false)
 	}
 	rbrack := p.expect(token.RBRACK)
 
@@ -1062,32 +1082,22 @@ func (p *parser) parseTypeParameters() *ast.FieldList {
 	return &ast.FieldList{Opening: lbrack, List: list, Closing: rbrack}
 }
 
-func (p *parser) parseParameters() *ast.FieldList {
+func (p *parser) parseParameters(result bool) *ast.FieldList {
 	if p.trace {
 		defer un(trace(p, "Parameters"))
 	}
 
-	lparen := p.expect(token.LPAREN)
-	var list []*ast.Field
-	if p.tok != token.RPAREN {
-		list = p.parseParameterList(nil, nil, token.RPAREN)
-	}
-	rparen := p.expect(token.RPAREN)
-
-	return &ast.FieldList{Opening: lparen, List: list, Closing: rparen}
-}
-
-func (p *parser) parseResult() *ast.FieldList {
-	if p.trace {
-		defer un(trace(p, "Result"))
-	}
-
-	if p.tok == token.LPAREN {
-		return p.parseParameters()
+	if !result || p.tok == token.LPAREN {
+		lparen := p.expect(token.LPAREN)
+		var list []*ast.Field
+		if p.tok != token.RPAREN {
+			list = p.parseParameterList(nil, nil, token.RPAREN, !result)
+		}
+		rparen := p.expect(token.RPAREN)
+		return &ast.FieldList{Opening: lparen, List: list, Closing: rparen}
 	}
 
-	typ := p.tryIdentOrType()
-	if typ != nil {
+	if typ := p.tryIdentOrType(); typ != nil {
 		list := make([]*ast.Field, 1)
 		list[0] = &ast.Field{Type: typ}
 		return &ast.FieldList{List: list}
@@ -1109,8 +1119,8 @@ func (p *parser) parseFuncType() *ast.FuncType {
 			p.error(tparams.Opening, "function type must have no type parameters")
 		}
 	}
-	params := p.parseParameters()
-	results := p.parseResult()
+	params := p.parseParameters(false)
+	results := p.parseParameters(true)
 
 	return &ast.FuncType{Func: pos, Params: params, Results: results}
 }
@@ -1138,13 +1148,13 @@ func (p *parser) parseMethodSpec() *ast.Field {
 				//
 				// Interface methods do not have type parameters. We parse them for a
 				// better error message and improved error recovery.
-				_ = p.parseParameterList(name0, nil, token.RBRACK)
+				_ = p.parseParameterList(name0, nil, token.RBRACK, false)
 				_ = p.expect(token.RBRACK)
 				p.error(lbrack, "interface method must have no type parameters")
 
 				// TODO(rfindley) refactor to share code with parseFuncType.
-				params := p.parseParameters()
-				results := p.parseResult()
+				params := p.parseParameters(false)
+				results := p.parseParameters(true)
 				idents = []*ast.Ident{ident}
 				typ = &ast.FuncType{
 					Func:    token.NoPos,
@@ -1173,8 +1183,8 @@ func (p *parser) parseMethodSpec() *ast.Field {
 		case p.tok == token.LPAREN:
 			// ordinary method
 			// TODO(rfindley) refactor to share code with parseFuncType.
-			params := p.parseParameters()
-			results := p.parseResult()
+			params := p.parseParameters(false)
+			results := p.parseParameters(true)
 			idents = []*ast.Ident{ident}
 			typ = &ast.FuncType{Func: token.NoPos, Params: params, Results: results}
 		default:
@@ -2578,7 +2588,7 @@ func (p *parser) parseGenericType(spec *ast.TypeSpec, openPos token.Pos, name0 *
 		defer un(trace(p, "parseGenericType"))
 	}
 
-	list := p.parseParameterList(name0, typ0, token.RBRACK)
+	list := p.parseParameterList(name0, typ0, token.RBRACK, false)
 	closePos := p.expect(token.RBRACK)
 	spec.TypeParams = &ast.FieldList{Opening: openPos, List: list, Closing: closePos}
 	if p.tok == token.ASSIGN {
@@ -2775,7 +2785,7 @@ func (p *parser) parseFuncDecl() *ast.FuncDecl {
 
 	var recv *ast.FieldList
 	if p.tok == token.LPAREN {
-		recv = p.parseParameters()
+		recv = p.parseParameters(false)
 	}
 
 	ident := p.parseIdent()
@@ -2790,8 +2800,8 @@ func (p *parser) parseFuncDecl() *ast.FuncDecl {
 			tparams = nil
 		}
 	}
-	params := p.parseParameters()
-	results := p.parseResult()
+	params := p.parseParameters(false)
+	results := p.parseParameters(true)
 
 	var body *ast.BlockStmt
 	switch p.tok {

src/go/parser/short_test.go

@@ -190,6 +190,14 @@ var invalids = []string{
 	`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() }`,
 
+	// variadic parameter lists
+	`package p; func f(a, b ... /* ERROR "can only use ... with final parameter" */ int)`,
+	`package p; func f(a ... /* ERROR "can only use ... with final parameter" */ int, b int)`,
+	`package p; func f(... /* ERROR "can only use ... with final parameter" */ int, int)`,
+	`package p; func f() (... /* ERROR "invalid use of ..." */ int)`,
+	`package p; func f() (a, b ... /* ERROR "invalid use of ..." */ int)`,
+	`package p; func f[T ... /* ERROR "invalid use of ..." */ C]()() {}`,
+
 	// generic code
 	`package p; type _[_ any] int; var _ = T[] /* ERROR "expected operand" */ {}`,
 	`package p; var _ func[ /* ERROR "must have no type parameters" */ T any](T)`,

src/go/types/expr.go

@@ -1006,9 +1006,8 @@ func (check *Checker) exprInternal(T *target, x *operand, e ast.Expr, hint Type)
 		check.ident(x, e, nil, false)
 
 	case *ast.Ellipsis:
-		// ellipses are handled explicitly where they are legal
-		// (array composite literals and parameter lists)
-		check.error(e, BadDotDotDotSyntax, "invalid use of '...'")
+		// ellipses are handled explicitly where they are valid
+		check.error(e, InvalidSyntaxTree, "invalid use of ...")
 		goto Error
 
 	case *ast.BasicLit:

src/go/types/signature.go

@@ -364,7 +364,7 @@ func (check *Checker) collectParams(list *ast.FieldList, variadicOk bool) (names
 			if variadicOk && i == len(list.List)-1 && len(field.Names) <= 1 {
 				variadic = true
 			} else {
-				check.softErrorf(t, MisplacedDotDotDot, "can only use ... with final parameter in list")
+				check.softErrorf(t, InvalidSyntaxTree, "invalid use of ...")
 				// ignore ... and continue
 			}
 		}

src/go/types/typexpr.go

@@ -322,10 +322,8 @@ func (check *Checker) typInternal(e0 ast.Expr, def *TypeName) (T Type) {
 		// report error if we encountered [...]
 
 	case *ast.Ellipsis:
-		// dots are handled explicitly where they are legal
-		// (array composite literals and parameter lists)
-		check.error(e, InvalidDotDotDot, "invalid use of '...'")
-		check.use(e.Elt)
+		// dots are handled explicitly where they are valid
+		check.error(e, InvalidSyntaxTree, "invalid use of ...")
 
 	case *ast.StructType:
 		typ := new(Struct)

src/internal/types/errors/code_string.go

@@ -719,10 +719,7 @@ const (
 
 	// MisplacedDotDotDot occurs when a "..." is used somewhere other than the
 	// final argument in a function declaration.
-	//
-	// Example:
-	// 	func f(...int, int)
-	MisplacedDotDotDot
+	_ // not used anymore (error reported by parser)
 
 	_ // InvalidDotDotDotOperand was removed.
 

src/internal/types/errors/codes.go

@@ -326,9 +326,9 @@ func issue28281b(a, b int, c ...int)
 func issue28281c(a, b, c ... /* ERROR "can only use ... with final parameter" */ int)
 func issue28281d(... /* ERROR "can only use ... with final parameter" */ int, int)
 func issue28281e(a, b, c  ... /* ERROR "can only use ... with final parameter" */ int, d int)
-func issue28281f(... /* ERROR "can only use ... with final parameter" */ int, ... /* ERROR "can only use ... with final parameter" */ int, int)
-func (... /* ERROR "invalid use of '...'" */ TT) f()
-func issue28281g() (... /* ERROR "can only use ... with final parameter" */ TT)
+func issue28281f(... /* ERROR "can only use ... with final parameter" */ int, ... int, int)
+func (... /* ERROR "invalid use of ..." */ TT) f()
+func issue28281g() (... /* ERROR "invalid use of ..." */ TT)
 
 // Issue #26234: Make various field/method lookup errors easier to read by matching cmd/compile's output
 func issue26234a(f *syn.Prog) {