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_attribute] doesn't work on trait methods #42493

Closed
alexcrichton opened this issue Jun 6, 2017 · 7 comments · Fixed by #44089
Closed

#[proc_macro_attribute] doesn't work on trait methods #42493

alexcrichton opened this issue Jun 6, 2017 · 7 comments · Fixed by #44089
Assignees
Labels
A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-bug Category: This is a bug.

Comments

@alexcrichton
Copy link
Member

Given this procedural macro:

#![feature(proc_macro)]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn foo(attr: TokenStream, item: TokenStream) -> TokenStream {
    println!("attr: {}", attr);
    println!("item: {}", item);
    item
}

and this crate:

#![feature(proc_macro)]

extern crate foo;

use foo::foo;

trait Trait {
    #[foo]
    fn trait_function(&self);
    #[foo]
    fn trait_function_default(&self) {}
}

fn main() {}

Building:

$ cargo +nightly build
   Compiling foo v0.1.0 (file:///home/alex/code/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.73 secs

I saw no output :(

I find that #[foo] is resolved, it just doesn't seem to get expanded!

@jseyfried
Copy link
Contributor

cc @abonander

@abonander
Copy link
Contributor

I saw this earlier and I've been thinking about it. I don't remember anything special in the invocation collector concerning trait methods though. Isn't there some codepath that strips attributes from trait methods, stuff like #[inline] that doesn't make sense on them?

@jseyfried
Copy link
Contributor

Not that I know of -- I'll do some debugging to figure out where the issue is (resolve, syntax, etc.).

@Mark-Simulacrum Mark-Simulacrum added the A-macros-2.0 Area: Declarative macros 2.0 (#39412) label Jun 23, 2017
@alexcrichton
Copy link
Member Author

Ok I've debugged this a bit and I think I've narrowed it down to resolve. The attribute is first resolved as Determinacy::Undetermined and then resolved as Determinacy::Determined which triggers this branch to I think fill in a "dummy" invocation, which ends up effectively stripping the item. Still trying to figure out why this isn't resolved right.

@alexcrichton
Copy link
Member Author

In attempting to debug further I think I'm pretty far out of my league here. I think what's happening is that resolution is attempting to resolve foo in the "trait's namespace", but that predictably fails and it in theory should go up to the parent module namespace and then do resolution there again, right? What I've found though is that hygienic_lexical_parent is returning None because while the trait does have a parent which presumably points to the crate's module they both have an expansion of Mark(0). The loop in hygienic_lexical_parent currently enforces parent_expansion != module_expansion which means that it can't go up one level.

Does that ring any bells @jseyfried or @abonander? Or maybe this is all a red herring?

@jseyfried
Copy link
Contributor

@alexcrichton

that resolution is attempting to resolve foo in the "trait's namespace"

Haven't looked into this yet, but this explanation makes sense.

The loop in hygienic_lexical_parent currently enforces parent_expansion != module_expansion which means that it can't go up one level.

I believe the issue here is that hygienic_lexical_parent shouldn't be called with a trait module.
hygienic_lexical_parent enforces parent_expansion != module_expansion for mod modules since in the normal, non-hygienic case, modules don't have lexical parents (except for the prelude).

That is,

mod foo {
    fn f() {}
    mod bar {
        fn g() { f() } // `f` doesn't resolve here
    }
}

However, due to hygiene, the following resolves:

mod foo {
    fn f() {}
    m!(f); // Due to hygiene, this `f` reliably resolves to the above `f` ...
}

macro m($f:ident) {
    mod bar {
        fn g() { $f } // ... no matter where it ends up being used in the macro definition.
        // fn $f() {} // (unless the macro definition explicitly shadows it like this)
    }
}

If this issue is as I expect, I can fix quickly tomorrow.

@alexcrichton
Copy link
Member Author

Ah ok, I'll leave you to it as I suspect you'll find it much more quickly than I will! If you run out of time though just let me know and I will resume digging!

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 25, 2017
This commit fixes procedural macro attributes being attached to trait methods,
ensuring that they get resolved and expanded as other procedural macro
attributes. The bug here was that `current_module` on the resolver was
accidentally set to be a trait when it's otherwise only ever expecting a
`mod`/block module. The actual fix here came from @jseyfried, I'm just helping
to land it in the compiler!

Closes rust-lang#42493
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 30, 2017
rustc: Fix proc_macro expansions on trait methods

This commit fixes procedural macro attributes being attached to trait methods,
ensuring that they get resolved and expanded as other procedural macro
attributes. The bug here was that `current_module` on the resolver was
accidentally set to be a trait when it's otherwise only ever expecting a
`mod`/block module. The actual fix here came from @jseyfried, I'm just helping
to land it in the compiler!

Closes rust-lang#42493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants