-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Treat ; as a terminator rather part of a glued expression #18744
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
Conversation
…her in an expression
@@ -132,6 +132,7 @@ impl<'a, S: Copy> TtIter<'a, S> { | |||
} | |||
('-' | '!' | '*' | '/' | '&' | '%' | '^' | '+' | '<' | '=' | '>' | '|', '=', _) | |||
| ('-' | '=' | '>', '>', _) | |||
| (_, _, Some(';')) |
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.
This looks like it should't change anything for the semicolon token, so that points to a more general bug in my eyes with our glueing infra structure
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.
This is needed because we incorrectly treat ; as a possible glued token, when it shouldn't be as it's an expression terminator. It does fix the bug in the repro mentioned above.
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.
To clarify, why this works is that we do a 3-token at a time lookahead, and we see :%; which without this matches the rule for % followed by : followed by % (which is allowed because it's not forbidden by the match expression).
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 see, it is clear that this entire part is messy and needs to be redone at some point (likely by #17830) but given this improves things right now lets merge it, thanks!
…er bug in MBE Specifically, rust-lang#18744 was the PR that was supposed to fix the old bug, but it fixed it incorrectly (and didn't add a test!) The underlying reason was that we marked metavariables in expansions as joint if they were joint in the macro call, which is incorrect. This wrong fix causes other bug, rust-lang#19497, which this PR fixes by removing the old (incorrect) fix.
…er bug in MBE Specifically, rust-lang#18744 was the PR that was supposed to fix the old bug, but it fixed it incorrectly (and didn't add a test!) The underlying reason was that we marked metavariables in expansions as joint if they were joint in the macro call, which is incorrect. This wrong fix causes other bug, rust-lang#19497, which this PR fixes by removing the old (incorrect) fix.
…er bug in MBE Specifically, rust-lang#18744 was the PR that was supposed to fix the old bug, but it fixed it incorrectly (and didn't add a test!) The underlying reason was that we marked metavariables in expansions as joint if they were joint in the macro call, which is incorrect. This wrong fix causes other bug, rust-lang#19497, which this PR fixes by removing the old (incorrect) fix.
The following code fails in rust-analyzer, but passes in rustc, using the "log" crate with the "kv" feature enabled:
This fails, because :%; is treated as a single glued token, rather than the :% glued expression followed by a ; terminator.
Fix this by not gluing ; to match rustc behavior.