Skip to content

Commit 334f2fc

Browse files
findleyrfindleyr
committed
[dev.typeparams] go/*: switch from ListExpr to MultiIndexExpr
When instantiating a generic type or function with multiple type arguments, we need to represent an index expression with multiple indexes in the AST. Previous to this CL this was done with a new ast.ListExpr node, which allowed packing multiple expressions into a single ast.Expr. This compositional pattern can be both inefficient and cumbersome to work with, and introduces a new node type that only exists to augment the meaning of an existing node type. By comparison, other specializations of syntax are given distinct nodes in go/ast, for example variations of switch or for statements, so the use of ListExpr was also (arguably) inconsistent. This CL removes ListExpr, and instead adds a MultiIndexExpr node, which is exactly like IndexExpr but allows for multiple index arguments. This requires special handling for this new node type, but a new wrapper in the typeparams helper package largely mitigates this special handling. Change-Id: I65eb29c025c599bae37501716284dc7eb953b2ad Reviewed-on: https://go-review.googlesource.com/c/go/+/327149 Trust: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Go Bot <gobot@golang.org>
1 parent 6b85a21 commit 334f2fc

File tree

14 files changed

+170
-167
lines changed

14 files changed

+170
-167
lines changed

‎src/go/ast/ast.go‎

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,15 @@ type (
344344
Rbrack token.Pos // position of "]"
345345
}
346346

347+
// A MultiIndexExpr node represents an expression followed by multiple
348+
// indices.
349+
MultiIndexExpr struct {
350+
X Expr // expression
351+
Lbrack token.Pos // position of "["
352+
Indices []Expr // index expressions
353+
Rbrack token.Pos // position of "]"
354+
}
355+
347356
// A SliceExpr node represents an expression followed by slice indices.
348357
SliceExpr struct {
349358
X Expr // expression
@@ -374,13 +383,6 @@ type (
374383
Rparen token.Pos // position of ")"
375384
}
376385

377-
// A ListExpr node represents a list of expressions separated by commas.
378-
// ListExpr nodes are used as index in IndexExpr nodes representing type
379-
// or function instantiations with more than one type argument.
380-
ListExpr struct {
381-
ElemList []Expr
382-
}
383-
384386
// A StarExpr node represents an expression of the form "*" Expression.
385387
// Semantically it could be a unary "*" expression, or a pointer type.
386388
//
@@ -494,21 +496,16 @@ func (x *CompositeLit) Pos() token.Pos {
494496
func (x *ParenExpr) Pos() token.Pos { return x.Lparen }
495497
func (x *SelectorExpr) Pos() token.Pos { return x.X.Pos() }
496498
func (x *IndexExpr) Pos() token.Pos { return x.X.Pos() }
499+
func (x *MultiIndexExpr) Pos() token.Pos { return x.X.Pos() }
497500
func (x *SliceExpr) Pos() token.Pos { return x.X.Pos() }
498501
func (x *TypeAssertExpr) Pos() token.Pos { return x.X.Pos() }
499502
func (x *CallExpr) Pos() token.Pos { return x.Fun.Pos() }
500-
func (x *ListExpr) Pos() token.Pos {
501-
if len(x.ElemList) > 0 {
502-
return x.ElemList[0].Pos()
503-
}
504-
return token.NoPos
505-
}
506-
func (x *StarExpr) Pos() token.Pos { return x.Star }
507-
func (x *UnaryExpr) Pos() token.Pos { return x.OpPos }
508-
func (x *BinaryExpr) Pos() token.Pos { return x.X.Pos() }
509-
func (x *KeyValueExpr) Pos() token.Pos { return x.Key.Pos() }
510-
func (x *ArrayType) Pos() token.Pos { return x.Lbrack }
511-
func (x *StructType) Pos() token.Pos { return x.Struct }
503+
func (x *StarExpr) Pos() token.Pos { return x.Star }
504+
func (x *UnaryExpr) Pos() token.Pos { return x.OpPos }
505+
func (x *BinaryExpr) Pos() token.Pos { return x.X.Pos() }
506+
func (x *KeyValueExpr) Pos() token.Pos { return x.Key.Pos() }
507+
func (x *ArrayType) Pos() token.Pos { return x.Lbrack }
508+
func (x *StructType) Pos() token.Pos { return x.Struct }
512509
func (x *FuncType) Pos() token.Pos {
513510
if x.Func.IsValid() || x.Params == nil { // see issue 3870
514511
return x.Func
@@ -533,21 +530,16 @@ func (x *CompositeLit) End() token.Pos { return x.Rbrace + 1 }
533530
func (x *ParenExpr) End() token.Pos { return x.Rparen + 1 }
534531
func (x *SelectorExpr) End() token.Pos { return x.Sel.End() }
535532
func (x *IndexExpr) End() token.Pos { return x.Rbrack + 1 }
533+
func (x *MultiIndexExpr) End() token.Pos { return x.Rbrack + 1 }
536534
func (x *SliceExpr) End() token.Pos { return x.Rbrack + 1 }
537535
func (x *TypeAssertExpr) End() token.Pos { return x.Rparen + 1 }
538536
func (x *CallExpr) End() token.Pos { return x.Rparen + 1 }
539-
func (x *ListExpr) End() token.Pos {
540-
if len(x.ElemList) > 0 {
541-
return x.ElemList[len(x.ElemList)-1].End()
542-
}
543-
return token.NoPos
544-
}
545-
func (x *StarExpr) End() token.Pos { return x.X.End() }
546-
func (x *UnaryExpr) End() token.Pos { return x.X.End() }
547-
func (x *BinaryExpr) End() token.Pos { return x.Y.End() }
548-
func (x *KeyValueExpr) End() token.Pos { return x.Value.End() }
549-
func (x *ArrayType) End() token.Pos { return x.Elt.End() }
550-
func (x *StructType) End() token.Pos { return x.Fields.End() }
537+
func (x *StarExpr) End() token.Pos { return x.X.End() }
538+
func (x *UnaryExpr) End() token.Pos { return x.X.End() }
539+
func (x *BinaryExpr) End() token.Pos { return x.Y.End() }
540+
func (x *KeyValueExpr) End() token.Pos { return x.Value.End() }
541+
func (x *ArrayType) End() token.Pos { return x.Elt.End() }
542+
func (x *StructType) End() token.Pos { return x.Fields.End() }
551543
func (x *FuncType) End() token.Pos {
552544
if x.Results != nil {
553545
return x.Results.End()
@@ -570,10 +562,10 @@ func (*CompositeLit) exprNode() {}
570562
func (*ParenExpr) exprNode() {}
571563
func (*SelectorExpr) exprNode() {}
572564
func (*IndexExpr) exprNode() {}
565+
func (*MultiIndexExpr) exprNode() {}
573566
func (*SliceExpr) exprNode() {}
574567
func (*TypeAssertExpr) exprNode() {}
575568
func (*CallExpr) exprNode() {}
576-
func (*ListExpr) exprNode() {}
577569
func (*StarExpr) exprNode() {}
578570
func (*UnaryExpr) exprNode() {}
579571
func (*BinaryExpr) exprNode() {}

‎src/go/ast/walk.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ func Walk(v Visitor, node Node) {
116116
Walk(v, n.X)
117117
Walk(v, n.Index)
118118

119+
case *MultiIndexExpr:
120+
Walk(v, n.X)
121+
for _, index := range n.Indices {
122+
Walk(v, index)
123+
}
124+
119125
case *SliceExpr:
120126
Walk(v, n.X)
121127
if n.Low != nil {
@@ -138,11 +144,6 @@ func Walk(v Visitor, node Node) {
138144
Walk(v, n.Fun)
139145
walkExprList(v, n.Args)
140146

141-
case *ListExpr:
142-
for _, elem := range n.ElemList {
143-
Walk(v, elem)
144-
}
145-
146147
case *StarExpr:
147148
Walk(v, n.X)
148149

‎src/go/internal/typeparams/typeparams.go‎

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,40 +7,54 @@ package typeparams
77
import (
88
"fmt"
99
"go/ast"
10+
"go/token"
1011
)
1112

1213
const Enabled = true
1314

14-
func PackExpr(list []ast.Expr) ast.Expr {
15-
switch len(list) {
15+
func PackIndexExpr(x ast.Expr, lbrack token.Pos, exprs []ast.Expr, rbrack token.Pos) ast.Expr {
16+
switch len(exprs) {
1617
case 0:
17-
// Return an empty ListExpr here, rather than nil, as IndexExpr.Index must
18-
// never be nil.
19-
// TODO(rFindley) would a BadExpr be more appropriate here?
20-
return &ast.ListExpr{}
18+
panic("internal error: PackIndexExpr with empty expr slice")
2119
case 1:
22-
return list[0]
20+
return &ast.IndexExpr{
21+
X: x,
22+
Lbrack: lbrack,
23+
Index: exprs[0],
24+
Rbrack: rbrack,
25+
}
2326
default:
24-
return &ast.ListExpr{ElemList: list}
27+
return &ast.MultiIndexExpr{
28+
X: x,
29+
Lbrack: lbrack,
30+
Indices: exprs,
31+
Rbrack: rbrack,
32+
}
2533
}
2634
}
2735

28-
// TODO(gri) Should find a more efficient solution that doesn't
29-
// require introduction of a new slice for simple
30-
// expressions.
31-
func UnpackExpr(x ast.Expr) []ast.Expr {
32-
if x, _ := x.(*ast.ListExpr); x != nil {
33-
return x.ElemList
34-
}
35-
if x != nil {
36-
return []ast.Expr{x}
37-
}
38-
return nil
36+
// IndexExpr wraps an ast.IndexExpr or ast.MultiIndexExpr into the
37+
// MultiIndexExpr interface.
38+
//
39+
// Orig holds the original ast.Expr from which this IndexExpr was derived.
40+
type IndexExpr struct {
41+
Orig ast.Expr // the wrapped expr, which may be distinct from MultiIndexExpr below.
42+
*ast.MultiIndexExpr
3943
}
4044

41-
func IsListExpr(n ast.Node) bool {
42-
_, ok := n.(*ast.ListExpr)
43-
return ok
45+
func UnpackIndexExpr(n ast.Node) *IndexExpr {
46+
switch e := n.(type) {
47+
case *ast.IndexExpr:
48+
return &IndexExpr{e, &ast.MultiIndexExpr{
49+
X: e.X,
50+
Lbrack: e.Lbrack,
51+
Indices: []ast.Expr{e.Index},
52+
Rbrack: e.Rbrack,
53+
}}
54+
case *ast.MultiIndexExpr:
55+
return &IndexExpr{e, e}
56+
}
57+
return nil
4458
}
4559

4660
func Get(n ast.Node) *ast.FieldList {

‎src/go/parser/parser.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ func (p *parser) parseArrayFieldOrTypeInstance(x *ast.Ident) (*ast.Ident, ast.Ex
600600
}
601601

602602
// x[P], x[P1, P2], ...
603-
return nil, &ast.IndexExpr{X: x, Lbrack: lbrack, Index: typeparams.PackExpr(args), Rbrack: rbrack}
603+
return nil, typeparams.PackIndexExpr(x, lbrack, args, rbrack)
604604
}
605605

606606
func (p *parser) parseFieldDecl() *ast.Field {
@@ -991,7 +991,7 @@ func (p *parser) parseMethodSpec() *ast.Field {
991991
p.exprLev--
992992
}
993993
rbrack := p.expectClosing(token.RBRACK, "type argument list")
994-
typ = &ast.IndexExpr{X: ident, Lbrack: lbrack, Index: typeparams.PackExpr(list), Rbrack: rbrack}
994+
typ = typeparams.PackIndexExpr(ident, lbrack, list, rbrack)
995995
}
996996
case p.tok == token.LPAREN:
997997
// ordinary method
@@ -1178,7 +1178,6 @@ func (p *parser) parseTypeInstance(typ ast.Expr) ast.Expr {
11781178
}
11791179

11801180
opening := p.expect(token.LBRACK)
1181-
11821181
p.exprLev++
11831182
var list []ast.Expr
11841183
for p.tok != token.RBRACK && p.tok != token.EOF {
@@ -1192,7 +1191,17 @@ func (p *parser) parseTypeInstance(typ ast.Expr) ast.Expr {
11921191

11931192
closing := p.expectClosing(token.RBRACK, "type argument list")
11941193

1195-
return &ast.IndexExpr{X: typ, Lbrack: opening, Index: typeparams.PackExpr(list), Rbrack: closing}
1194+
if len(list) == 0 {
1195+
p.errorExpected(closing, "type argument list")
1196+
return &ast.IndexExpr{
1197+
X: typ,
1198+
Lbrack: opening,
1199+
Index: &ast.BadExpr{From: opening + 1, To: closing},
1200+
Rbrack: closing,
1201+
}
1202+
}
1203+
1204+
return typeparams.PackIndexExpr(typ, opening, list, closing)
11961205
}
11971206

11981207
func (p *parser) tryIdentOrType() ast.Expr {
@@ -1455,7 +1464,7 @@ func (p *parser) parseIndexOrSliceOrInstance(x ast.Expr) ast.Expr {
14551464
}
14561465

14571466
// instance expression
1458-
return &ast.IndexExpr{X: x, Lbrack: lbrack, Index: typeparams.PackExpr(args), Rbrack: rbrack}
1467+
return typeparams.PackIndexExpr(x, lbrack, args, rbrack)
14591468
}
14601469

14611470
func (p *parser) parseCallOrConversion(fun ast.Expr) *ast.CallExpr {
@@ -1557,6 +1566,7 @@ func (p *parser) checkExpr(x ast.Expr) ast.Expr {
15571566
panic("unreachable")
15581567
case *ast.SelectorExpr:
15591568
case *ast.IndexExpr:
1569+
case *ast.MultiIndexExpr:
15601570
case *ast.SliceExpr:
15611571
case *ast.TypeAssertExpr:
15621572
// If t.Type == nil we have a type assertion of the form
@@ -1646,7 +1656,7 @@ func (p *parser) parsePrimaryExpr() (x ast.Expr) {
16461656
return
16471657
}
16481658
// x is possibly a composite literal type
1649-
case *ast.IndexExpr:
1659+
case *ast.IndexExpr, *ast.MultiIndexExpr:
16501660
if p.exprLev < 0 {
16511661
return
16521662
}

‎src/go/printer/nodes.go‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -871,17 +871,15 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) {
871871
// TODO(gri): should treat[] like parentheses and undo one level of depth
872872
p.expr1(x.X, token.HighestPrec, 1)
873873
p.print(x.Lbrack, token.LBRACK)
874-
// Note: we're a bit defensive here to handle the case of a ListExpr of
875-
// length 1.
876-
if list := typeparams.UnpackExpr(x.Index); len(list) > 0 {
877-
if len(list) > 1 {
878-
p.exprList(x.Lbrack, list, depth+1, commaTerm, x.Rbrack, false)
879-
} else {
880-
p.expr0(list[0], depth+1)
881-
}
882-
} else {
883-
p.expr0(x.Index, depth+1)
884-
}
874+
p.expr0(x.Index, depth+1)
875+
p.print(x.Rbrack, token.RBRACK)
876+
877+
case *ast.MultiIndexExpr:
878+
// TODO(gri): as for IndexExpr, should treat [] like parentheses and undo
879+
// one level of depth
880+
p.expr1(x.X, token.HighestPrec, 1)
881+
p.print(x.Lbrack, token.LBRACK)
882+
p.exprList(x.Lbrack, x.Indices, depth+1, commaTerm, x.Rbrack, false)
885883
p.print(x.Rbrack, token.RBRACK)
886884

887885
case *ast.SliceExpr:

0 commit comments

Comments
 (0)