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

Grouping hygiene broken when copying TokenTrees to a new TokenStream #74036

Closed
de-vri-es opened this issue Jul 4, 2020 · 3 comments
Closed
Labels
C-bug Category: This is a bug.

Comments

@de-vri-es
Copy link
Contributor

de-vri-es commented Jul 4, 2020

We encountered an issue with the assert2 crate where grouping hygiene did not survive a proc macro.

use assert2::assert;

fn main() {
    macro_rules! bla {
        ($e:expr) => { assert!($e * $e == 4); }
    }
    bla!(1 + 1); // Fails, compares 1 + 1 * 1 + 1 == 4, should be (1 + 1) * (1 + 1) == 4
}

See de-vri-es/assert2-rs#14 for the original issue.


We digged a little further and came up with a minimal example here: de-vri-es/proc-macro-grouping-hygiene-bug. It contains two proc macros, one that just clones a TokenStream, and one that creates a new one from the input:

use proc_macro::TokenStream;

#[proc_macro]
pub fn identity(tokens: TokenStream) -> TokenStream {
	tokens.clone()
}

#[proc_macro]
pub fn manual_identity(tokens: TokenStream) -> TokenStream {
	let mut output = TokenStream::new();
	output.extend(tokens);
	output
}

Then we run these tests:

use proc_macro_grouping_hygiene_bug::*;

macro_rules! foo1 {
	($expr:expr) => { identity!($expr * $expr) };
}

macro_rules! foo2 {
	($expr:expr) => { manual_identity!($expr * $expr) };
}

#[test]
fn test_identity() {
	assert_eq!(foo1!(1 + 1), 4); // OK, gives 4.
	assert_eq!(foo2!(1 + 1), 4); // FAILS, gives 3.
}

We expected both assertions to pass, but the second one fails. The same problem occurs when converting a syn::Expr into a token stream again (which is how we originally found it).

This seems like a very serious issue to me.

@de-vri-es de-vri-es added the C-bug Category: This is a bug. label Jul 4, 2020
@petrochenkov
Copy link
Contributor

This is an instance of #67062.

@de-vri-es
Copy link
Contributor Author

Ah, you are correct. Should I close this issue?

@petrochenkov
Copy link
Contributor

Yes, I've left a comment on that issue, closing.

m-ou-se added a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 4, 2020
de-vri-es pushed a commit to de-vri-es/assert2-rs that referenced this issue Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants