-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add note for identifier following by array literal error #108222
Conversation
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some nits.
cc @pnkfelix who suggested this message -- To be quite honest, I'm not sure if the parser really benefits from such a bespoke recovery. I agree that the error message you originally hit was confusing, but this seems like it'll probably never get hit by someone else in production often -- at least with the way it's being implemented currently. Do you have thoughts? I don't want to block this parser change if you disagree.
48fdf43
to
93a3409
Compare
☔ The latest upstream changes (presumably #108488) made this pull request unmergeable. Please resolve the merge conflicts. |
r? @pnkfelix |
So, I first off want to apologize for how long its taken me to get around to this.
Looking at this PR, I'm inclined to agree with Michael's intuition above. This is a lot of dedicated code for a very narrow case. It is not the approach I expected to see here. I was expecting someting more along the lines of: First establish some kind of state saying "we currently think we are inside an index-expr (i.e. a pair of based on the rest of Michael's comment, its possible that I should accept that this issue just isn't going to be fixed at all. I'm not quite ready to give up to that level, but I am willing to say that we don't want a fix that looks like this PR. @obeis , if you're interested in working together to figure out what the right solution here looks like, please do ping me on Zulip (preferably in a fresh public topic under the #t-compiler stream) and we can try to hash this out. But in the meantime, I'm going to close this PR because I don't think it will land in this state. (Also, @obeis , thank you for your patience and for your contribution.) |
Closes #107518