[go] go/parser: don't parse a nil IndexExpr.Index
Robert Findley (Gerrit)
Robert Findley has uploaded this change for review.
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
---
M src/go/ast/walk.go
M src/go/internal/typeparams/notypeparams.go
M src/go/internal/typeparams/typeparams.go
M src/go/parser/parser.go
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/go/ast/walk.go b/src/go/ast/walk.go
index ac1395f..9224264 100644
--- a/src/go/ast/walk.go
+++ b/src/go/ast/walk.go
@@ -112,11 +112,7 @@
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 a8c25ac..2ceafaa 100644
--- a/src/go/internal/typeparams/notypeparams.go
+++ b/src/go/internal/typeparams/notypeparams.go
@@ -15,8 +15,6 @@
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 66f66af..871e95d 100644
--- a/src/go/internal/typeparams/typeparams.go
+++ b/src/go/internal/typeparams/typeparams.go
@@ -17,7 +17,10 @@
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 36a044e..3965641 100644
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -1095,6 +1095,7 @@
}
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"))
}
To view, visit change 315851. To unsubscribe, or for help writing mail filters, visit settings.
Robert Findley (Gerrit)
Attention is currently required from: Robert Griesemer.
1 comment:
Patchset:
Gentle ping. I'd like to get this merged soonish.
To view, visit change 315851. To unsubscribe, or for help writing mail filters, visit settings.
Robert Griesemer (Gerrit)
Attention is currently required from: Robert Findley.
Patch set 1:Code-Review +2Trust +1
2 comments:
Patchset:
LGTM but see my comment.
File src/go/internal/typeparams/typeparams.go:
Patch Set #1, Line 23:
ListExprI'm concerned that this will cause other problems with code that assumes a ListExpr is never empty.
To view, visit change 315851. To unsubscribe, or for help writing mail filters, visit settings.
Robert Findley (Gerrit)
1 comment:
File src/go/internal/typeparams/typeparams.go:
Patch Set #1, Line 23:
ListExprI'm concerned that this will cause other problems with code that assumes a ListExpr is never empty.
Ack. I think for now I'll merge this, but work on that in a follow-up (perhaps we should just parse a BadExpr after all).
For now, the priority is ensuring that invariants are not broken in 1.17.
To view, visit change 315851. To unsubscribe, or for help writing mail filters, visit settings.
Robert Findley (Gerrit)
Robert Findley submitted this change.
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 <rfin...@google.com>
Trust: Robert Griesemer <g...@golang.org>
Run-TryBot: Robert Findley <rfin...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Robert Griesemer <g...@golang.org>
To view, visit change 315851. To unsubscribe, or for help writing mail filters, visit settings.