Skip to content
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

reject macros with empty repetitions #36721

Merged
merged 1 commit into from
Sep 27, 2016
Merged

Conversation

TimNN
Copy link
Contributor

@TimNN TimNN commented Sep 25, 2016

Fixes #5067 by checking the lhs of macro_rules! for repetitions which could match an empty token tree.

@rust-highfive
Copy link
Collaborator

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -358,6 +358,10 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt,
valid &= check_rhs(cx, rhs);
}

for lhs in &lhses {
valid &= check_lhs_no_empty_seq(cx, &[lhs.clone()])
}
Copy link
Member

Choose a reason for hiding this comment

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

could use lhses.iter().all() instead of the for loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The for loop was mainly used to use the same style as the loop of the rhses directly above.

Also .iter().all() would abort early, I think, which means that not all lhses would be checked, ie. not all errors we could report would be reported.

Copy link
Member

Choose a reason for hiding this comment

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

ok, good point on not reporting all errors. Could you add a comment to explain that please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@nrc
Copy link
Member

nrc commented Sep 25, 2016

r=me with the for loop replaced

@nrc
Copy link
Member

nrc commented Sep 25, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 25, 2016

📌 Commit d06a9ca has been approved by nrc

@bors
Copy link
Contributor

bors commented Sep 25, 2016

☔ The latest upstream changes (presumably #36616) made this pull request unmergeable. Please resolve the merge conflicts.

@TimNN
Copy link
Contributor Author

TimNN commented Sep 26, 2016

Rebased.

@nrc
Copy link
Member

nrc commented Sep 26, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 26, 2016

📌 Commit 51ea050 has been approved by nrc

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 26, 2016
reject macros with empty repetitions

Fixes rust-lang#5067 by checking the lhs of `macro_rules!` for repetitions which could match an empty token tree.
sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 27, 2016
reject macros with empty repetitions

Fixes rust-lang#5067 by checking the lhs of `macro_rules!` for repetitions which could match an empty token tree.
bors added a commit that referenced this pull request Sep 27, 2016
@bors bors merged commit 51ea050 into rust-lang:master Sep 27, 2016
@TimNN TimNN deleted the infinite-emptiness branch September 28, 2016 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants