Skip to content

Allow semicolon after closure within parentheses in macros #103224

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Oct 19, 2022

#88546 added some parsing logic that if we're parsing a closure, and we're within parentheses, and a semicolon follows, then we must be parsing something erroneous like: f(|| a; b), so it replaces the closure body with an error expression. However, it's valid to parse those tokens if we're within a macro, as in #103222.

This is a bit unsatisfying fix. Is there a more robust way of checking that we're within a macro?

I would also be open to removing this "It is likely that the closure body is a block but where the braces have been removed" check altogether at the expense of more verbose errors, since it seems very suspicious in the first place...

Fixes #103222.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 19, 2022
@rust-highfive
Copy link
Contributor

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2022
@@ -2051,6 +2051,7 @@ impl<'a> Parser<'a> {

if self.token.kind == TokenKind::Semi
&& matches!(self.token_cursor.frame.delim_sp, Some((Delimiter::Parenthesis, _)))
&& self.subparser_name.is_none()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a helper method for checks like this to make it more readable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can if you want, but given that this is a hack, I don't want to necessarily encourage more use-cases of this within the parser. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there are no other places using this. I guess add a comment explaining what this means and then r= me.

@fee1-dead fee1-dead added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 22, 2022
@compiler-errors compiler-errors force-pushed the semi-after-closure-in-macro branch from d8afdc3 to 3d035a0 Compare October 22, 2022 06:52
@fee1-dead

This comment was marked as outdated.

@compiler-errors compiler-errors force-pushed the semi-after-closure-in-macro branch from 3d035a0 to af02c1e Compare October 22, 2022 06:59
@compiler-errors compiler-errors force-pushed the semi-after-closure-in-macro branch from af02c1e to 3d7b1f0 Compare October 22, 2022 07:00
@fee1-dead
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 22, 2022

📌 Commit 3d7b1f0 has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 22, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102602 (Slightly tweak comments wrt `lint_overflowing_range_endpoint`)
 - rust-lang#103190 (rustdoc: render bounds of cross-crate GAT params)
 - rust-lang#103224 (Allow semicolon after closure within parentheses in macros)
 - rust-lang#103280 ((rust-lang#102929) Implement `String::leak` (attempt 2))
 - rust-lang#103329 (Add a forgotten check for NonNull::new_unchecked's precondition)
 - rust-lang#103346 (Adjust argument type for mutable with_metadata_of (rust-lang#75091))
 - rust-lang#103360 (Reduce false positives in msys2 detection)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ada5011 into rust-lang:master Oct 22, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 22, 2022
@compiler-errors compiler-errors deleted the semi-after-closure-in-macro branch November 2, 2022 02:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler Panic on specific macro expansion
5 participants