-
Notifications
You must be signed in to change notification settings - Fork 18k
go/parser: audit error recovery for go1.17 #46403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@findleyr We're close to RC1, and this issue is a release blocker. Any updates? |
Thanks. This is mostly procedural, though I'm aware of one minor parser change that must be made. I can make sure to close this out by this Monday the 21st, does that work? |
Sure, next Monday is fine. Thanks. |
Change https://golang.org/cl/329791 mentions this issue: |
In addition to #46855, and
These seem to be reasonable changes in behavior related to the refactoring of function signature parsing (to allow for type parameter lists), and index expressions (to allow for type instantiation). In the cases I've seen, the 1.17 recovery is generally equivalent to or better than the 1.16 error recovery, though of course this is subjective. It's also hard to filter out the noise of these classes of errors without rewriting the 1.16 parser. Therefore, I think we should close this bug after CL 329791 is merged. There may be more classes of differences, but I don't have an easy way to detect them. |
To be consistent with Go 1.16, and to preserve as much information in the AST as possible, parse an ast.IndexExpr with BadExpr Index for the invalid expression a[]. A go/types test had to be adjusted to account for an additional error resulting from this change. We don't have a lot of test coverage for parser error recovery, so rather than write an ad-hoc test for this issue, add a new go/types test that checks that the indexed operand is used. Updates #46403 Change-Id: I21e6ff4179746aaa50e530d4091fded450e69824 Reviewed-on: https://go-review.googlesource.com/c/go/+/329791 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>
With all the changes to go/parser merged from
dev.typeparams
, at least some error recovery behavior has changed in go 1.17. For example,a[]
parses toa[BadExpr]
on go1.16, but justBadExpr
on go1.17.Error recovery in the parser is best-effort, but we should audit changes to ensure they are acceptable. We should probably avoid changes such as the example above that lose information.
CC @griesemer
The text was updated successfully, but these errors were encountered: