go/parser: don't parse a nil IndexExpr.Index

When parsing type parameters, an empty type instantiation was parsed as
an IndexExpr with nil Index. This should be considered a breaking change
to parsing: ast.Walk previously assumed that Index was non-nil.

Back out the nil check in ast.Walk, and for now pack an empty argument
list as a non-nil ListExpr with nil Elems.

Alternatives considered:
 - Parsing the entire index expression as a BadExpr: this led to
   inferior errors while type checking.
 - Parsing the Index as a BadExpr: this seems reasonable, but encodes
   strictly less information into the AST.

We may want to opt for one of these alternatives in the future, but for
now let's just fix the breaking change.

Change-Id: I93f2b89641692ac014b8ee98bfa031ed3477afb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/315851
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

4 files changed, 6 insertions, 8 deletions

diff --git a/src/go/ast/walk.go b/src/go/ast/walk.go
index ac1395fafd..9224264e29 100644
--- a/src/go/ast/walk.go
+++ b/src/go/ast/walk.go
@@ -112,11 +112,7 @@ func Walk(v Visitor, node Node) {
 
 	case *IndexExpr:
 		Walk(v, n.X)
-		// n.Index may be nil for invalid type instantiation expressions, e.g.
-		// var x T[].
-		if n.Index != nil {
-			Walk(v, n.Index)
-		}
+		Walk(v, n.Index)
 
 	case *SliceExpr:
 		Walk(v, n.X)
diff --git a/src/go/internal/typeparams/notypeparams.go b/src/go/internal/typeparams/notypeparams.go
index a8c25ac2b1..2ceafaac1c 100644
--- a/src/go/internal/typeparams/notypeparams.go
+++ b/src/go/internal/typeparams/notypeparams.go
@@ -15,8 +15,6 @@ const Enabled = false
 
 func PackExpr(list []ast.Expr) ast.Expr {
 	switch len(list) {
-	case 0:
-		return nil
 	case 1:
 		return list[0]
 	default:
diff --git a/src/go/internal/typeparams/typeparams.go b/src/go/internal/typeparams/typeparams.go
index 66f66afb28..871e95d998 100644
--- a/src/go/internal/typeparams/typeparams.go
+++ b/src/go/internal/typeparams/typeparams.go
@@ -17,7 +17,10 @@ const Enabled = true
 func PackExpr(list []ast.Expr) ast.Expr {
 	switch len(list) {
 	case 0:
-		return nil
+		// Return an empty ListExpr here, rather than nil, as IndexExpr.Index must
+		// never be nil.
+		// TODO(rFindley) would a BadExpr be more appropriate here?
+		return &ast.ListExpr{}
 	case 1:
 		return list[0]
 	default:
diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go
index 36a044e3a2..3965641713 100644
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -1095,6 +1095,7 @@ func (p *parser) parseChanType() *ast.ChanType {
 }
 
 func (p *parser) parseTypeInstance(typ ast.Expr) ast.Expr {
+	assert(p.parseTypeParams(), "parseTypeInstance while not parsing type params")
 	if p.trace {
 		defer un(trace(p, "TypeInstance"))
 	}