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

proc_macro API: Expose macro_rules hygiene #64690

Merged
merged 1 commit into from
Oct 4, 2019
Merged

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Sep 22, 2019

Proc macros do not have direct access to our oldest and most stable hygiene kind - macro_rules hygiene.

To emulate it macro authors have to go through two steps - first generate a temporary macro_rules item (using a derive, at least until #64035 is merged), then generate a macro call to that item. Popular crates like proc_macro_hack use this trick to generate hygienic identifiers from proc macros.

I'd say that these workarounds with nested macro definitions have more chances to hit some corner cases in our hygiene system, in which we don't have full confidence.
So, let's provide a direct access to macro_rules hygiene instead.

This PR does that by adding a new method Span::mixed_site (bikeshedding is welcome) in addition to existing Span::call_site (stable) and Span::def_site (unstable).
Identifiers with this span resolve at def-site in for local variables, labels and $crate, and resolve at call-site for everything else, i.e. exactly like identifiers produced by macro_rules.

This API addition opens the way to stabilizing proc macros in expression positions (#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.
(macro_rules expanded in expression position, on the other hand, are stable since 1.0 and widely used.)

r? @dtolnay @alexcrichton

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2019
@petrochenkov petrochenkov added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 22, 2019
@alexcrichton
Copy link
Member

Seems plausible to me! @dtolnay would know much more about the need for this than I though

@SergioBenitez
Copy link
Contributor

This API addition opens the way to stabilizing proc macros in expression positions (#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.

How does this PR do that? Is the plan to enjoin mixed-site hygiene on proc-macros that expand to expressions?

@petrochenkov
Copy link
Contributor Author

Is the plan to enjoin mixed-site hygiene on proc-macros that expand to expressions?

Exactly.

@Alexendoo
Copy link
Member

Ping from triage, any updates? @dtolnay

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I like this.

What would be your recommendation to macro authors on when this is the right span to use? Just everywhere as a default span in any expression-position macro?

@petrochenkov
Copy link
Contributor Author

I'd say everywhere until Span::def_site() arrives (unless you specifically need an unhygienic local or something, but that's an exceptional case).
Partial hygiene is still a better default than no hygiene at all, there's a reason why macro_rules use it.

@dtolnay
Copy link
Member

dtolnay commented Oct 2, 2019

I will try to test whether changing the default span of quote! from call_site to mixed_site on sufficiently new compilers can possibly break anything that's currently stable. The alternative would be adding a macro like quote_expr! that is shorthand for quote_spanned!(Span::mixed_site()=>...).

LGTM after the documentation that Sergio pointed out is fixed.

@petrochenkov petrochenkov 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 2, 2019
@Centril
Copy link
Contributor

Centril commented Oct 3, 2019

cc @pnkfelix

@petrochenkov
Copy link
Contributor Author

@bors r=dtolnay

@bors
Copy link
Contributor

bors commented Oct 3, 2019

📌 Commit d1310dc has been approved by dtolnay

@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 3, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 3, 2019
proc_macro API: Expose `macro_rules` hygiene

Proc macros do not have direct access to our oldest and most stable hygiene kind - `macro_rules` hygiene.

To emulate it macro authors have to go through two steps - first generate a temporary `macro_rules` item (using a derive, at least until rust-lang#64035 is merged), then generate a macro call to that item. Popular crates like [proc_macro_hack](https://crates.io/crates/proc-macro-hack) use this trick to generate hygienic identifiers from proc macros.

I'd say that these workarounds with nested macro definitions have more chances to hit some corner cases in our hygiene system, in which we don't have full confidence.
So, let's provide a direct access to `macro_rules` hygiene instead.

This PR does that by adding a new method `Span::mixed_site` (bikeshedding is welcome) in addition to existing `Span::call_site` (stable) and `Span::def_site` (unstable).
Identifiers with this span resolve at def-site in for local variables, labels and `$crate`, and resolve at call-site for everything else, i.e. exactly like identifiers produced by `macro_rules`.

This API addition opens the way to stabilizing proc macros in expression positions (rust-lang#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.
(`macro_rules` expanded in expression position, on the other hand, are stable since 1.0 and widely used.)

r? @dtolnay @alexcrichton
bors added a commit that referenced this pull request Oct 3, 2019
Rollup of 5 pull requests

Successful merges:

 - #64690 (proc_macro API: Expose `macro_rules` hygiene)
 - #64749 (Fix most remaining Polonius test differences)
 - #64938 (Avoid ICE on ReFree region on where clause)
 - #64999 (extract expected return type for async fn generators)
 - #65037 (`#[track_caller]` feature gate (RFC 2091))

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Oct 3, 2019

⌛ Testing commit d1310dc with merge 8d7c6e33e4bd8f6ed8953991f3992af618cbdbd1...

@Centril
Copy link
Contributor

Centril commented Oct 3, 2019

@bors retry -- prioritizing other PRs to see which one caused failure in #65053 (comment) :(

Centril added a commit to Centril/rust that referenced this pull request Oct 3, 2019
proc_macro API: Expose `macro_rules` hygiene

Proc macros do not have direct access to our oldest and most stable hygiene kind - `macro_rules` hygiene.

To emulate it macro authors have to go through two steps - first generate a temporary `macro_rules` item (using a derive, at least until rust-lang#64035 is merged), then generate a macro call to that item. Popular crates like [proc_macro_hack](https://crates.io/crates/proc-macro-hack) use this trick to generate hygienic identifiers from proc macros.

I'd say that these workarounds with nested macro definitions have more chances to hit some corner cases in our hygiene system, in which we don't have full confidence.
So, let's provide a direct access to `macro_rules` hygiene instead.

This PR does that by adding a new method `Span::mixed_site` (bikeshedding is welcome) in addition to existing `Span::call_site` (stable) and `Span::def_site` (unstable).
Identifiers with this span resolve at def-site in for local variables, labels and `$crate`, and resolve at call-site for everything else, i.e. exactly like identifiers produced by `macro_rules`.

This API addition opens the way to stabilizing proc macros in expression positions (rust-lang#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.
(`macro_rules` expanded in expression position, on the other hand, are stable since 1.0 and widely used.)

r? @dtolnay @alexcrichton
tmandry added a commit to tmandry/rust that referenced this pull request Oct 3, 2019
proc_macro API: Expose `macro_rules` hygiene

Proc macros do not have direct access to our oldest and most stable hygiene kind - `macro_rules` hygiene.

To emulate it macro authors have to go through two steps - first generate a temporary `macro_rules` item (using a derive, at least until rust-lang#64035 is merged), then generate a macro call to that item. Popular crates like [proc_macro_hack](https://crates.io/crates/proc-macro-hack) use this trick to generate hygienic identifiers from proc macros.

I'd say that these workarounds with nested macro definitions have more chances to hit some corner cases in our hygiene system, in which we don't have full confidence.
So, let's provide a direct access to `macro_rules` hygiene instead.

This PR does that by adding a new method `Span::mixed_site` (bikeshedding is welcome) in addition to existing `Span::call_site` (stable) and `Span::def_site` (unstable).
Identifiers with this span resolve at def-site in for local variables, labels and `$crate`, and resolve at call-site for everything else, i.e. exactly like identifiers produced by `macro_rules`.

This API addition opens the way to stabilizing proc macros in expression positions (rust-lang#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.
(`macro_rules` expanded in expression position, on the other hand, are stable since 1.0 and widely used.)

r? @dtolnay @alexcrichton
tmandry added a commit to tmandry/rust that referenced this pull request Oct 3, 2019
proc_macro API: Expose `macro_rules` hygiene

Proc macros do not have direct access to our oldest and most stable hygiene kind - `macro_rules` hygiene.

To emulate it macro authors have to go through two steps - first generate a temporary `macro_rules` item (using a derive, at least until rust-lang#64035 is merged), then generate a macro call to that item. Popular crates like [proc_macro_hack](https://crates.io/crates/proc-macro-hack) use this trick to generate hygienic identifiers from proc macros.

I'd say that these workarounds with nested macro definitions have more chances to hit some corner cases in our hygiene system, in which we don't have full confidence.
So, let's provide a direct access to `macro_rules` hygiene instead.

This PR does that by adding a new method `Span::mixed_site` (bikeshedding is welcome) in addition to existing `Span::call_site` (stable) and `Span::def_site` (unstable).
Identifiers with this span resolve at def-site in for local variables, labels and `$crate`, and resolve at call-site for everything else, i.e. exactly like identifiers produced by `macro_rules`.

This API addition opens the way to stabilizing proc macros in expression positions (rust-lang#54727), for which use of call-site hygiene or workarounds with temporary items would be quite unfortunate.
(`macro_rules` expanded in expression position, on the other hand, are stable since 1.0 and widely used.)

r? @dtolnay @alexcrichton
bors added a commit that referenced this pull request Oct 3, 2019
Rollup of 11 pull requests

Successful merges:

 - #61879 (Stabilize todo macro)
 - #64675 (Deprecate `#![plugin]` & `#[plugin_registrar]`)
 - #64690 (proc_macro API: Expose `macro_rules` hygiene)
 - #64706 (add regression test for #60218)
 - #64741 (Prevent rustdoc feature doctests)
 - #64842 (Disallow Self in type param defaults of ADTs)
 - #65004 (Replace mentions of IRC with Discord)
 - #65018 (Set RUST_BACKTRACE=0 in tests that include a backtrace in stderr)
 - #65055 (Add long error explanation for E0556)
 - #65056 (Make visit projection iterative)
 - #65057 (typo: fix typo in E0392)

Failed merges:

r? @ghost
@bors bors merged commit d1310dc into rust-lang:master Oct 4, 2019
@CAD97

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants