Proc macro hygiene regression

[SimonSapin]

Something in the range bb42071...f9b0897, likely #46343 (CC @jseyfried), broke Servo.

We have a #[dom_struct] attribute implemented in a dom_struct crate like this:

#[proc_macro_attribute]
pub fn dom_struct(args: TokenStream, input: TokenStream) -> TokenStream {
    if !args.is_empty() {
        panic!("#[dom_struct] takes no arguments");
    }
    let attributes = quote! {
        #[derive(DenyPublicFields, DomObject, JSTraceable, MallocSizeOf)]
        #[must_root]
        #[repr(C)]
    };
    iter::once(attributes).chain(iter::once(input)).collect()
}

Each of the derives is defined in a respective crate. The script crate depends on all of these crates and uses #[dom_struct]. Some of the derives generate code that reference items defined in script. For example, #[derive(DomObject)] implements the script::dom::bindings::refector::DomObject trait.

Since rustc 1.24.0-nightly (f9b0897 2017-12-02), every use of #[dom_struct] fails with:

error[E0433]: failed to resolve. Could not find `js` in `{{root}}`
  --> components/script/dom/attr.rs:28:1
   |
28 | #[dom_struct]
   | ^^^^^^^^^^^^^ Could not find `js` in `{{root}}`

error[E0433]: failed to resolve. Could not find `dom` in `{{root}}`
  --> components/script/dom/attr.rs:28:1
   |
28 | #[dom_struct]
   | ^^^^^^^^^^^^^ Could not find `dom` in `{{root}}`

error[E0433]: failed to resolve. Could not find `js` in `{{root}}`
  --> components/script/dom/attr.rs:28:1
   |
28 | #[dom_struct]
   | ^^^^^^^^^^^^^ Could not find `js` in `{{root}}`

error[E0433]: failed to resolve. Could not find `malloc_size_of` in `{{root}}`
  --> components/script/dom/attr.rs:28:1
   |
28 | #[dom_struct]
   | ^^^^^^^^^^^^^ Could not find `malloc_size_of` in `{{root}}`

I suppose that these errors come from code generated by the derives, and that {{root}} refers to the root of the dom_struct crate where the #[derive(…)] tokens come from. Indeed, the js and dom are not an cannot be available there, they’re in the script crate which depends on dom_struct.

We can work around this by erasing hygiene data in the #[derive(…)] tokens:

--- a/components/dom_struct/lib.rs
+++ b/components/dom_struct/lib.rs
@@ -17,7 +17,9 @@ pub fn dom_struct(args: TokenStream, input: TokenStream) -> TokenStream {
     let attributes = quote! {
         #[derive(DenyPublicFields, DomObject, JSTraceable, MallocSizeOf)]
         #[must_root]
         #[repr(C)]
     };
+    let attributes = attributes.to_string().parse().unwrap();
     iter::once(attributes).chain(iter::once(input)).collect()
 }

… but this seems like a valid pattern that shouldn’t necessitate such hacks.

[Mark-Simulacrum]

This also broke perf.rlo (due to breaking servo).

[SimonSapin]

rust-lang/rustc-perf#171 applies the work around to the servo copy in perf.rlo.

[nrc]

This seems like correct behaviour. The derive names must be in scope where the macro is defined, not where it is used. I don't recall how derives are looked up though, so I'm not sure if that mechanism works correctly with the proper hygiene here - we should re-open or open another issue if that doesn't work.

Stripping the hygiene info as in the work-around (which is actually setting all the hygiene info to the macro use site) is not recommended.

[nrc]

Urgh, sorry, I misunderstood what is going on here - it seems that we're looking up and expanding the derives fine. Given that the derives (unhygienic) are being expanded inside a hygienic context, I'm not even sure where I would expect them to look for their names.

[SimonSapin]

is not recommended

I do realize this is a hack that I’d rather was not needed, but this is the only way I’ve found to make this existing code compile with a newer compiler, which unblocks unrelated work.

[victorporof]

This broke rsx. Latest nightly that works is from Dec 1.

This doesn't work anymore:

let foo = 42;
...
my_proc_macro! { foo }

[victorporof]

A more explicative use case for this in case of rsx would be the following:

let stylesheet = ...;
let node = rsx! {
  <view style={stylesheet.get(".root")}>
      Hello world!
  </view>
};

where rsx! is a procedural macro.

[bhuztez]

This also breaks my code

I have a proc_macro like this:

#![feature(proc_macro)]
#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::{quote, TokenStream, TokenTree, TokenTreeIter, TokenNode, Delimiter};

fn parse(iter: &mut TokenTreeIter) -> TokenStream {
    iter.next();
    iter.next();
    if let Some(TokenTree{span: _, kind: TokenNode::Group(Delimiter::None, stream)}) = iter.next() {
        quote!($stream)
    } else {
        TokenStream::empty()
    }
}

#[proc_macro]
pub fn my_macro(stream: TokenStream) -> TokenStream {
    parse(&mut stream.into_iter())
}

now I got error[E0425]: cannot find value x in this scope

#![feature(proc_macro)]
extern crate my_macro;

pub use my_macro::my_macro;

macro_rules! m {
    ($($arg:expr),*) => (
        $crate::my_macro!($crate $(, $arg)*)
    )
}

fn main() {
    let x = 1usize;
    let y = m!(&x);
    println!("{}", y);
}

[parched]

I think I'm hitting this too, here's the simplest repro I could do

#![feature(proc_macro)]
extern crate proc_macro;
use proc_macro::TokenStream;
use std::str::FromStr;

#[proc_macro]
pub fn add(token_stream: TokenStream) -> TokenStream {
    TokenStream::from_str(&token_stream.to_string().replace(",", "+")).unwrap()
}

#![feature(proc_macro)]
extern crate add;
use add::add;

#[test]
fn test_add() {
    let a = 1;
    let b = 2;
    let c = add!(a, b);
    assert_eq!(3, c);
}

error[E0425]: cannot find value `b` in this scope
  --> tests/simple.rs:11:13
   |
11 |     let c = add!(a, b);
   |             ^^^^^^^^^^ not found in this scope

[semtexzv]

Also hitting this error in my tests of proc_macro. Example :
https://gist.github.com/semtexzv/77facc8bf206b1d585214cea65757b1f

[M-Adoo]

I m hinting this too , same like with @parched demo code.

[jseyfried]

I'm working on a PR to fix this now.

[rushmorem]

Is there any workaround for this? One of my libraries is depending on proc-macro-hack which is now broken.

[SimonSapin]

@rushmorem In the specific code originally in this issue, I’ve worked around by replacing let attributes = quote! {…}; with let attributes = quote! {…}.to_string().parse().unwrap();. This works around hygiene by "erasing" the span information from these tokens.

[rushmorem]

@SimonSapin Thank you. I will try that.

[rail44]

I wrote function to make macro can use call_site variable, like

fn into_call_site_span(stream: TokenStream) -> TokenStream {
    let call_site_span = Span::call_site();
    let iter = stream.into_iter().map(|mut tree| {
        match tree {
            TokenTree::Group(g) => {
                TokenTree::Group(Group::new(g.delimiter(), into_call_site_span(g.stream())))
            }
            _ => {
                tree.set_span(call_site_span);
                tree
            }
        }
    });
    TokenStream::from_iter(iter)
}

[jeremydavis519]

@rail44 That's exactly the workaround I needed! Do you mind if I use it?

[rail44]

@jeremydavis519 Of course.

[joshlf]

Still getting this issue with feature(proc_macro_hygiene); I assume that's expected behavior? The code in question:

Proc macro

#[proc_macro]
pub fn ip(input: TokenStream) -> TokenStream {
    // format and remove all spaces, or else IP parsing will fail
    let s = format!("{}", input).replace(" ", "");
    match ip_helper(&s) {
        Ok(stream) => stream,
        Err(_) => panic!("invalid IP address or subnet: {}", s),
    }
}

fn ip_helper(s: &str) -> Result<TokenStream, ()> {
    Ok(match s.parse() {
        Ok(IpAddr::V4(v4)) => {
            let octets = v4.octets();
            quote!(::ip::Ipv4Addr::new([#(#octets),*])).into()
        }
        Ok(IpAddr::V6(v6)) => {
            let octets = v6.octets();
            quote!(::ip::Ipv6Addr::new([#(#octets),*])).into()
        }
        Err(_) => {
            // try to parse as a subnet before returning error
            if !s.contains("/") {
                return Err(());
            }
            let parts: Vec<&str> = s.split("/").collect();
            if parts.len() != 2 {
                return Err(());
            }
            let ip: IpAddr = parts[0].parse().map_err(|_| ())?;
            let prefix: u8 = parts[1].parse().map_err(|_| ())?;
            match ip {
                IpAddr::V4(v4) => {
                    if prefix > 32 {
                        return Err(());
                    }
                    let octets = v4.octets();
                    quote!(::ip::Subnet::new(::ip::Ipv4Addr::new([#(#octets),*]), #prefix)).into()
                }
                IpAddr::V6(v6) => {
                    if prefix > 128 {
                        return Err(());
                    }
                    let octets = v6.octets();
                    quote!(::ip::Subnet::new(::ip::Ipv6Addr::new([#(#octets),*]), #prefix)).into()
                }
            }
        }
    })
}

Calling code:

pub const GATEWAY_ADDR: Ipv4Addr = ip!(10.123.0.3);

Error:

error[E0433]: failed to resolve: could not find `ip` in `{{root}}`
   --> src/wire/testdata.rs:301:40
    |
301 |     pub const GATEWAY_ADDR: Ipv4Addr = ip!(10.123.0.3);
    | 

[Enselic]

Triage: I see no use of any nightly exclusive feature, so this seems to be a stable-stable regression. Labeling as such.