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

Fix hygiene in various macros #204

Merged
merged 1 commit into from
Nov 21, 2017
Merged

Conversation

alexcrichton
Copy link
Member

No description provided.

Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

For my own edification, could you say more about what this PR is fixing? :)

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 21, 2017

Thanks for the quick fix, could you fix the version of syn, quote, etc. to a particular commit that works? We could do a single upgrade once syn releases the next version but until then you are probably the only person that can fix things quickly when a new commit in syn breaks our build bots because the new syn/quote/proc_macro2 crates are basically undocumented :/

@alexcrichton
Copy link
Member Author

@BurntSushi sure!

This is mostly related to rust-lang/rust#45934 and the regession here was caused by dtolnay/quote#55. What ends up happening here is that on nightly, which we're using, procedural macros deal with tokens and each token has a "span". The spans not only indicate what position in the file you're at (e.g. for error reporting) but they also indicate hygiene for macros and whatnot.

So before dtolnay/quote#55 the quote! macro worked by calling stringify!(..) on each token passed to the macro and then calling parse() to convert it to a proc_macro::TokenStream. That implicitly tagged it with a span which... I'm not entirely sure what span but it in general is told to "skip hygiene" at that point.

The macros here, for example, unhygienically reference the stdsimd_test crate as well as the cfg_feature_enabled! macro. For example the macros expand to stdsimd_test, but they're referencing the definition in the call site, not in the definition site (the procedural macro crate). Similarly cfg_feature_enabled! is defined in the library, not in the assert_instr_macro crate.

So what happened with dtolnay/quote#55 is that it stopped using the parse'd Span by default and instead started using Span::def_site(). That in turn caused differences in hygiene which broke these macros. Instead now this PR switches to using Span::call_site() for these generated tokens which effectively goes back to what we had before (non-hygienic)

@alexcrichton
Copy link
Member Author

@gnzlbg perhaps yeah, but presumably that'd be with a Cargo.lock rather than pins. I'd ideally like to not do that as this breaks very rarely.

@alexcrichton alexcrichton merged commit 4c53e76 into rust-lang:master Nov 21, 2017
@alexcrichton alexcrichton deleted the fix branch November 21, 2017 18:54
@BurntSushi
Copy link
Member

@alexcrichton Wow, crazy. Thanks for the explanation!

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.

3 participants