Proc macro hygiene regression

16e5a7c
Opened by Simon Sapin at 2024-12-12 23:10:10

Something in the range https://github.com/rust-lang/rust/compare/bb42071f6...f9b0897c5, likely https://github.com/rust-lang/rust/pull/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 (f9b0897c5 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.

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

    Mark Rousskov at 2017-12-04 17:11:48

  2. https://github.com/rust-lang-nursery/rustc-perf/pull/171 applies the work around to the servo copy in perf.rlo.

    Simon Sapin at 2017-12-04 17:18:10

  3. 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.

    Nick Cameron at 2017-12-07 21:53:43

  4. 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.

    Nick Cameron at 2017-12-07 22:10:35

  5. 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.

    Simon Sapin at 2017-12-07 22:16:17

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

    This doesn't work anymore:

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

    Victor Porof at 2017-12-11 20:00:44

  7. 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.

    Victor Porof at 2017-12-12 11:20:40

  8. 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);
    }
    

    bhuztez at 2017-12-29 15:53:51

  9. 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
    

    James Duley at 2018-01-05 08:54:42

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

    Michal Hornický at 2018-01-17 08:28:11

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

    Adoo at 2018-01-17 15:37:07

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

    Jeffrey Seyfried at 2018-01-18 00:53:26

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

    Rushmore Mushambi at 2018-03-06 06:29:10

  14. @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.

    Simon Sapin at 2018-03-06 07:58:29

  15. @SimonSapin Thank you. I will try that.

    Rushmore Mushambi at 2018-03-06 08:21:30

  16. 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)
    }
    

    Satoshi Amemiya at 2018-04-15 07:20:11

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

    Jeremy Davis at 2018-04-28 19:06:16

  18. @jeremydavis519 Of course.

    Satoshi Amemiya at 2018-05-10 12:41:58

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

    <details> <summary>Proc macro</summary>
    #[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()
                    }
                }
            }
        })
    }
    
    </details>

    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);
        | 
    

    Joshua Liebow-Feeser at 2019-02-15 04:44:39

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

    Martin Nordholts at 2023-11-19 05:40:26