Skip to content

Commit

Permalink
Auto merge of #73293 - Aaron1011:feature/macro-rules-arg-capture, r=p…
Browse files Browse the repository at this point in the history
…etrochenkov

Always capture tokens for `macro_rules!` arguments

When we invoke a proc-macro, the `TokenStream` we pass to it may contain 'interpolated' AST fragments, represented by `rustc_ast::token::Nonterminal`. In order to correctly, pass a `Nonterminal` to a proc-macro, we need to have 'captured' its `TokenStream` at the time the AST was parsed.

Currently, we perform this capturing when attributes are present on items and expressions, since we will end up using a `Nonterminal` to pass the item/expr to any proc-macro attributes it is annotated with. However, `Nonterminal`s are also introduced by the expansion of metavariables in `macro_rules!` macros. Since these metavariables may be passed to proc-macros, we need to have tokens available to avoid the need to pretty-print and reparse (see #43081).

This PR unconditionally performs token capturing for AST items and expressions that are passed to a `macro_rules!` invocation. We cannot know in advance if captured item/expr will be passed to proc-macro, so this is needed to ensure that tokens will always be available when they are needed.

This ensures that proc-macros will receive tokens with proper `Spans` (both location and hygiene) in more cases. Like all work on #43081, this will cause regressions in proc-macros that were relying on receiving tokens with dummy spans.

In this case, Crater revealed only one regression: the [Pear](https://github.com/SergioBenitez/Pear) crate (a helper for [rocket](https://github.com/SergioBenitez/Rocket)), which was previously [fixed](SergioBenitez/Pear#25) as part of #73084.

This regression manifests itself as the following error:

```
[INFO] [stdout] error: proc macro panicked
[INFO] [stdout]    --> /opt/rustwide/cargo-home/registry/src/github.com-1ecc6299db9ec823/rocket_http-0.4.5/src/parse/uri/parser.rs:119:34
[INFO] [stdout]     |
[INFO] [stdout] 119 |             let path_and_query = pear_try!(path_and_query(is_pchar));
[INFO] [stdout]     |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[INFO] [stdout]     |
[INFO] [stdout]     = help: message: called `Option::unwrap()` on a `None` value
[INFO] [stdout]     = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
```

It can be fixed by running `cargo update -p pear`, which updates your `Cargo.lock` to use the latest version of Pear (which includes a bugfix for the regression).

Split out from #73084
  • Loading branch information
bors committed Jun 24, 2020
2 parents 0c04344 + 74599cd commit 3c90ae8
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 5 deletions.
29 changes: 25 additions & 4 deletions src/librustc_expand/mbe/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,18 +866,39 @@ fn parse_nt(p: &mut Parser<'_>, sp: Span, name: Symbol) -> Result<Nonterminal, (
}

fn parse_nt_inner<'a>(p: &mut Parser<'a>, sp: Span, name: Symbol) -> PResult<'a, Nonterminal> {
// Any `Nonterminal` which stores its tokens (currently `NtItem` and `NtExpr`)
// needs to have them force-captured here.
// A `macro_rules!` invocation may pass a captured item/expr to a proc-macro,
// which requires having captured tokens available. Since we cannot determine
// in advance whether or not a proc-macro will be (transitively) invoked,
// we always capture tokens for any `Nonterminal` which needs them.
Ok(match name {
sym::item => match p.parse_item()? {
Some(i) => token::NtItem(i),
None => return Err(p.struct_span_err(p.token.span, "expected an item keyword")),
sym::item => match p.collect_tokens(|this| this.parse_item())? {
(Some(mut item), tokens) => {
// If we captured tokens during parsing (due to outer attributes),
// use those.
if item.tokens.is_none() {
item.tokens = Some(tokens);
}
token::NtItem(item)
}
(None, _) => return Err(p.struct_span_err(p.token.span, "expected an item keyword")),
},
sym::block => token::NtBlock(p.parse_block()?),
sym::stmt => match p.parse_stmt()? {
Some(s) => token::NtStmt(s),
None => return Err(p.struct_span_err(p.token.span, "expected a statement")),
},
sym::pat => token::NtPat(p.parse_pat(None)?),
sym::expr => token::NtExpr(p.parse_expr()?),
sym::expr => {
let (mut expr, tokens) = p.collect_tokens(|this| this.parse_expr())?;
// If we captured tokens during parsing (due to outer attributes),
// use those.
if expr.tokens.is_none() {
expr.tokens = Some(tokens);
}
token::NtExpr(expr)
}
sym::literal => token::NtLiteral(p.parse_literal_maybe_minus()?),
sym::ty => token::NtTy(p.parse_ty()?),
// this could be handled like a token, since it is one
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_parse/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ impl<'a> Parser<'a> {
/// This restriction shouldn't be an issue in practice,
/// since this function is used to record the tokens for
/// a parsed AST item, which always has matching delimiters.
fn collect_tokens<R>(
pub fn collect_tokens<R>(
&mut self,
f: impl FnOnce(&mut Self) -> PResult<'a, R>,
) -> PResult<'a, (R, TokenStream)> {
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/proc-macro/auxiliary/first-second.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::{TokenStream, TokenTree, Group, Delimiter};

#[proc_macro_attribute]
pub fn first(_attr: TokenStream, item: TokenStream) -> TokenStream {
let tokens: TokenStream = "#[derive(Second)]".parse().unwrap();
let wrapped = TokenTree::Group(Group::new(Delimiter::None, item.into_iter().collect()));
tokens.into_iter().chain(std::iter::once(wrapped)).collect()
}

#[proc_macro_derive(Second)]
pub fn second(item: TokenStream) -> TokenStream {
TokenStream::new()
}
12 changes: 12 additions & 0 deletions src/test/ui/proc-macro/auxiliary/recollect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro]
pub fn recollect(tokens: TokenStream) -> TokenStream {
tokens.into_iter().collect()
}
48 changes: 48 additions & 0 deletions src/test/ui/proc-macro/auxiliary/weird-hygiene.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::{TokenStream, TokenTree, Group};

fn find_my_ident(tokens: TokenStream) -> Option<TokenStream> {
for token in tokens {
if let TokenTree::Ident(ident) = &token {
if ident.to_string() == "hidden_ident" {
return Some(vec![token].into_iter().collect())
}
} else if let TokenTree::Group(g) = token {
if let Some(stream) = find_my_ident(g.stream()) {
return Some(stream)
}
}
}
return None;
}


#[proc_macro_derive(WeirdDerive)]
pub fn weird_derive(item: TokenStream) -> TokenStream {
let my_ident = find_my_ident(item).expect("Missing 'my_ident'!");
let tokens: TokenStream = "call_it!();".parse().unwrap();
let final_call = tokens.into_iter().map(|tree| {
if let TokenTree::Group(g) = tree {
return Group::new(g.delimiter(), my_ident.clone()).into()
} else {
return tree
}
}).collect();
final_call
}

#[proc_macro]
pub fn recollect(item: TokenStream) -> TokenStream {
item.into_iter().collect()
}

#[proc_macro_attribute]
pub fn recollect_attr(_attr: TokenStream, mut item: TokenStream) -> TokenStream {
item.into_iter().collect()
}
22 changes: 22 additions & 0 deletions src/test/ui/proc-macro/capture-macro-rules-invoke.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// aux-build:test-macros.rs
// check-pass

extern crate test_macros;
use test_macros::recollect;

macro_rules! use_expr {
($expr:expr) => {
recollect!($expr)
}
}

#[allow(dead_code)]
struct Foo;
impl Foo {
#[allow(dead_code)]
fn use_self(self) {
drop(use_expr!(self));
}
}

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/proc-macro/macro-rules-derive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// aux-build:first-second.rs
// FIXME: The spans here are bad, see PR #73084

extern crate first_second;
use first_second::*;

macro_rules! produce_it {
($name:ident) => {
#[first] //~ ERROR cannot find type
struct $name {
field: MissingType
}
}
}

produce_it!(MyName);

fn main() {
println!("Hello, world!");
}
9 changes: 9 additions & 0 deletions src/test/ui/proc-macro/macro-rules-derive.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0412]: cannot find type `MissingType` in this scope
--> $DIR/macro-rules-derive.rs:9:9
|
LL | #[first]
| ^^^^^^^^ not found in this scope

error: aborting due to previous error

For more information about this error, try `rustc --explain E0412`.
48 changes: 48 additions & 0 deletions src/test/ui/proc-macro/weird-hygiene.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// aux-build:weird-hygiene.rs
// check-pass
// FIXME: This should actually error, see PR #73084

#![feature(stmt_expr_attributes)]
#![feature(proc_macro_hygiene)]

extern crate weird_hygiene;
use weird_hygiene::*;

macro_rules! other {
($tokens:expr) => {
macro_rules! call_it {
($outer_ident:ident) => {
macro_rules! inner {
() => {
$outer_ident;
}
}
}
}

#[derive(WeirdDerive)]
enum MyEnum {
Value = (stringify!($tokens + hidden_ident), 1).1
}

inner!();
}
}

macro_rules! invoke_it {
($token:expr) => {
#[recollect_attr] {
$token;
hidden_ident
}
}
}

fn main() {
// `other` and `invoke_it` are both macro_rules! macros,
// so it should be impossible for them to ever see `hidden_ident`,
// even if they invoke a proc macro.
let hidden_ident = "Hello1";
other!(50);
invoke_it!(25);
}

0 comments on commit 3c90ae8

Please sign in to comment.