go/parser, go/types: accept parameterized embedded struct fields

To be able to distinguish between a named field with a parenthesized
type `foo (T)` and an embedded parameterized type `foo(T)` in structs
we permit the use of parentheses in embedded types. For instance, the
latter example will need to be written as `(foo(T))`.

The parser now accepts any parenthesized embedded type liberally
and assumes that type-checking makes sure only permissible types
are accepted. As a result, parsing of field declarations is now
massively simpler. Also, removed an unused function.

The type-checker now complains about invalid embedded types (as
opposed to an invalid AST); it also now accepts embedded parameterized
types.

Change-Id: Ib8c6cbea9a0b739f66ba266fb5f8b71f91b7b64e

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)

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){}`,
 

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.
+

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())
+}

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
 	)
 }
 

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
 }