diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index e88bb0f8f3..56a1e6b09c 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -600,9 +600,7 @@ func (p *parser) parseLhsList() []ast.Expr { switch p.tok { case token.DEFINE: // lhs of a short variable declaration - // but doesn't enter scope until later: - // caller must call p.shortVarDecl(p.makeIdentList(list)) - // at appropriate time. + // but doesn't enter scope until later. case token.COLON: // lhs of a label declaration or a communication clause of a select // statement (parseLhsList is not called when parsing the case clause @@ -690,35 +688,6 @@ func (p *parser) parseArrayLen() ast.Expr { return len } -func (p *parser) makeIdentList(list []ast.Expr) []*ast.Ident { - idents := make([]*ast.Ident, len(list)) - for i, x := range list { - ident, isIdent := x.(*ast.Ident) - if !isIdent { - if _, isBad := x.(*ast.BadExpr); !isBad { - // only report error if it's a new one - p.errorExpected(x.Pos(), "identifier") - } - ident = &ast.Ident{NamePos: x.Pos(), Name: "_"} - } - idents[i] = ident - } - return idents -} - -func (p *parser) parseOptionalTag() (tag *ast.BasicLit) { - if p.trace { - defer un(trace(p, "OptionalTag")) - } - - if p.tok == token.STRING { - tag = &ast.BasicLit{ValuePos: p.pos, Kind: p.tok, Value: p.lit} - p.next() - } - - return -} - func (p *parser) parseFieldDecl(scope *ast.Scope) *ast.Field { if p.trace { defer un(trace(p, "FieldDecl")) @@ -726,18 +695,13 @@ func (p *parser) parseFieldDecl(scope *ast.Scope) *ast.Field { doc := p.leadComment - makeField := func(typ ast.Expr, tag *ast.BasicLit, names ...*ast.Ident) *ast.Field { - field := &ast.Field{Doc: doc, Names: names, Type: typ, Tag: tag, Comment: p.lineComment} - p.declare(field, nil, scope, ast.Var, names...) - return field - } - - switch p.tok { - case token.IDENT: + var names []*ast.Ident + var typ ast.Expr + if p.tok == token.IDENT { name := p.parseIdent() if p.tok == token.PERIOD || p.tok == token.STRING || p.tok == token.SEMICOLON || p.tok == token.RBRACE { - // T - var typ ast.Expr = name + // embedded type + typ = name if p.tok == token.PERIOD { typ = p.parseTypeName(name) // A "(" indicates a type parameter (or a syntax error). @@ -747,83 +711,31 @@ func (p *parser) parseFieldDecl(scope *ast.Scope) *ast.Field { } else { p.resolve(typ) } - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag) - } - names := []*ast.Ident{name} - for p.tok == token.COMMA { - p.next() - names = append(names, p.parseIdent()) - } - typ := p.parseType(true) - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag, names...) - - case token.LPAREN: - p.error(p.pos, "cannot parenthesize embedded type") - p.next() - if p.tok == token.MUL { - // (*T) tag - pos := p.pos - p.next() - base := p.parseTypeName(nil) - p.expect(token.RPAREN) - if _, ok := base.(*ast.Ident); ok { - p.resolve(base) - } - typ := &ast.StarExpr{Star: pos, X: base} - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag) - } else { - // (T) tag - typ := p.parseTypeName(nil) - p.expect(token.RPAREN) - if _, ok := typ.(*ast.Ident); ok { - p.resolve(typ) + // name1, name2, ... T + names = []*ast.Ident{name} + for p.tok == token.COMMA { + p.next() + names = append(names, p.parseIdent()) } - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag) + typ = p.parseType(true) } - - case token.MUL: - pos := p.pos - p.next() - if p.tok == token.LPAREN { - // *(T) tag - p.error(p.pos, "cannot parenthesize embedded type") - p.next() - base := p.parseTypeName(nil) - p.expect(token.RPAREN) - if _, ok := base.(*ast.Ident); ok { - p.resolve(base) - } - typ := &ast.StarExpr{Star: pos, X: base} - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag) - - } else { - // *T tag - base := p.parseTypeName(nil) - if _, ok := base.(*ast.Ident); ok { - p.resolve(base) - } - typ := &ast.StarExpr{Star: pos, X: base} - tag := p.parseOptionalTag() - p.expectSemi() - return makeField(typ, tag) - } - - default: - p.errorExpected(p.pos, "field name or embedded type") - p.advance(exprEnd) - return nil + } else { + // embedded type + typ = p.parseType(true) } + + var tag *ast.BasicLit + if p.tok == token.STRING { + tag = &ast.BasicLit{ValuePos: p.pos, Kind: p.tok, Value: p.lit} + p.next() + } + + p.expectSemi() + + field := &ast.Field{Doc: doc, Names: names, Type: typ, Tag: tag, Comment: p.lineComment} + p.declare(field, nil, scope, ast.Var, names...) + return field } func (p *parser) parseStructType() *ast.StructType { @@ -836,12 +748,7 @@ func (p *parser) parseStructType() *ast.StructType { scope := ast.NewScope(nil) // struct scope var list []*ast.Field for p.tok == token.IDENT || p.tok == token.MUL || p.tok == token.LPAREN { - // a field declaration cannot start with a '(' but we accept - // it here for more robust parsing and better error messages - // (parseFieldDecl will check and complain if necessary) - if f := p.parseFieldDecl(scope); f != nil { - list = append(list, f) - } + list = append(list, p.parseFieldDecl(scope)) } rbrace := p.expect(token.RBRACE) @@ -1129,8 +1036,7 @@ func (p *parser) parseMethodSpec(scope *ast.Scope) *ast.Field { doc := p.leadComment var idents []*ast.Ident var typ ast.Expr - switch p.tok { - case token.IDENT: + if p.tok == token.IDENT { x := p.parseTypeName(nil) if ident, isIdent := x.(*ast.Ident); isIdent && p.tok == token.LPAREN { // method @@ -1144,7 +1050,7 @@ func (p *parser) parseMethodSpec(scope *ast.Scope) *ast.Field { typ = x p.resolve(typ) } - case token.LPAREN: + } else { // embedded, possibly parameterized interface // (using the enclosing parentheses to distinguish it from a method declaration) typ = p.parseType(true) diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go index 7b249300f1..abca95fc4d 100644 --- a/src/go/parser/short_test.go +++ b/src/go/parser/short_test.go @@ -50,6 +50,11 @@ var valids = []string{ `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(type P) struct { P }`, `package p; type T(type P comparable) struct { P }`, @@ -191,7 +196,7 @@ 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; 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){}`, diff --git a/src/go/types/NOTES b/src/go/types/NOTES index ae94ed179f..b18f45ebe1 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -28,9 +28,7 @@ OPEN QUESTIONS - For len/cap(x) where x is of type parameter type and the bound contains arrays only, should the result be a constant? (right now it is not). What are the implications for alternative, non- monomorphizing implementation methods? -- Confirm that it's ok to use inference in missingMethod to compare parameterized methods -- Now that we allow parenthesized embedded interfaces, should we allow parenthesized embedded fields? - (probably yes, for symmetry and consistency). +- Confirm that it's ok to use inference in missingMethod to compare parameterized methods. - What does it mean to explicitly instantiate a contract with a non-type parameter argument? (e.g., contract C(T) { T int }; func _(type T C(int))(...) ... seems invalid. What are the rules?) - Should we be able to parenthesize embedded contracts like we allow for interfaces (currently this @@ -94,3 +92,8 @@ DESIGN/IMPLEMENTATION but we cannot call such a function as "f()(1)"; the empty type argument list causes problems. Document that we allow empty type parameter list declarations, but not empty actual type parameter lists. (We could allow them for types, but that doesn't seem consistent and probably is confusing). + +- 2/19/2020: We accept parenthesized embedded struct fields so we can distinguish between a named + field with a parenthesized type foo (T) and an embedded parameterized type (foo(T)), similarly + to interace embedding. + diff --git a/src/go/types/examples/types.go2 b/src/go/types/examples/types.go2 index 5c50b5c771..453b48cfdb 100644 --- a/src/go/types/examples/types.go2 +++ b/src/go/types/examples/types.go2 @@ -146,3 +146,30 @@ func _() { x.m1(true) x.m("foo") } + +// We accept parenthesized embedded struct fields so we can distinguish between +// a named field with a parenthesized type foo (T) and an embedded parameterized +// type (foo(T)), similarly to interace embedding. +// They still need to be valid embedded types after the parentheses are stripped +// (i.e., in contrast to interfaces, we cannot embed a struct literal). The name +// of the embedded field is derived as before, after stripping parentheses. +type _ struct { + (int8) + (*int16) + *(int32) + (*(int64)) + (((*(int)))) + (*(List(int))) + + int8 /* ERROR int8 redeclared */ + int /* ERROR int redeclared */ + * /* ERROR List redeclared */ List(int) + + ( /* ERROR invalid embedded field type */ [10]int) + ( /* ERROR invalid embedded field type */ []int) + ( /* ERROR invalid embedded field type */ struct{}) + ( /* ERROR invalid embedded field type */ map[int]string) + ( /* ERROR invalid embedded field type */ chan<- int) + ( /* ERROR invalid embedded field type */ interface{}) + ( /* ERROR invalid embedded field type */ func()) +} diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go index 51ee0b1c36..8b3c5c986e 100644 --- a/src/go/types/stdlib_test.go +++ b/src/go/types/stdlib_test.go @@ -184,6 +184,7 @@ func TestStdFixed(t *testing.T) { "issue31747.go", // go/types does not have constraints on language level (-lang=go1.12) (see #31793) "issue34329.go", // go/types does not have constraints on language level (-lang=go1.13) (see #31793) "bug251.go", // issue #34333 which was exposed with fix for #34151 + "bug299.go", // go/types permits parenthesized embedded fields ) } diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index b8fd02ca0a..27eead349f 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -842,12 +842,12 @@ func (check *Checker) structType(styp *Struct, e *ast.StructType) { } } else { // embedded field - // spec: "An embedded type must be specified as a type name T or as a pointer - // to a non-interface type name *T, and T itself may not be a pointer type." + // spec: "An embedded type must be specified as a (possibly parenthesized) type name T or + // as a pointer to a non-interface type name *T, and T itself may not be a pointer type." pos := f.Type.Pos() name := embeddedFieldIdent(f.Type) if name == nil { - check.invalidAST(pos, "embedded field type %s has no name", f.Type) + check.errorf(pos, "invalid embedded field type %s", f.Type) name = ast.NewIdent("_") name.NamePos = pos addInvalid(name, pos) @@ -902,6 +902,10 @@ func embeddedFieldIdent(e ast.Expr) *ast.Ident { } case *ast.SelectorExpr: return e.Sel + case *ast.CallExpr: + return embeddedFieldIdent(e.Fun) + case *ast.ParenExpr: + return embeddedFieldIdent(e.X) } return nil // invalid embedded field }