proc_macro_attribute doesn't work on extern functions

[mjbshaw]

Given a proc-macro crate named demo_macros, which contains a single nop_attribute procedural macro:

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

#[proc_macro_attribute]
pub fn nop_attribute(_: TokenStream, input: TokenStream) -> TokenStream {
  input
}

Given a lib crate named demo, which attempts to use the nop_attribute procedural macro on a function within an extern block:

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

extern {
  #[nop_attribute]
  pub fn fabs(_: f64) -> f64;
}

Attempting to cargo +nightly build this results in the following error:

   Compiling demo_macros v0.1.0 (file:///private/tmp/demo/demo_macros)
   Compiling demo v0.1.0 (file:///private/tmp/demo)
error[E0658]: The attribute `nop_attribute` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642)
 --> src/lib.rs:6:3
  |
6 |   #[nop_attribute]
  |   ^^^^^^^^^^^^^^^^
  |
  = help: add #![feature(custom_attribute)] to the crate attributes to enable

error: aborting due to previous error

If you want more information on this error, try using "rustc --explain E0658"
error: Could not compile `demo`.

To learn more, run the command again with --verbose

If I delete the extern block and change the code to just be #[nop_attribute] pub fn fabs(_: f64) -> f64 { 0.0 } then it compiles just fine. For some reason, attempting to apply a procedural macro attribute to an extern function doesn't work.

My rustc --version:

rustc 1.26.0-nightly (e026b59cf 2018-03-03)

---

You can fully reproduce the issue by executing the following sequence of shell commands:

$ cargo +nightly new demo_macros
$ cat >>  demo_macros/Cargo.toml << EOF
[lib]
proc-macro = true
EOF
$ cat >| demo_macros/src/lib.rs << EOF
#![feature(proc_macro)]
extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn nop_attribute(_: TokenStream, input: TokenStream) -> TokenStream {
  input
}
EOF
$ cargo +nightly init .
$ echo 'demo_macros = { path = "demo_macros" }' >> Cargo.toml
$ cat >| src/lib.rs << EOF
#![feature(proc_macro)]
extern crate demo_macros;
use demo_macros::nop_attribute;

extern {
  #[nop_attribute]
  pub fn fabs(_: f64) -> f64;
}
EOF
$ cargo +nightly build

[abonander]

I'm guessing that it's more like proc_macro_attribute doesn't work inside extern {} blocks. I'd expect it to work on extern functions with Rust bodies:

#[nop_attribute]
pub extern fn foo() {
    // ...
}

But not on other item types inside extern {} blocks:

extern {
    #[nop_attribute]
    static mut FOO: u32;
}

My guess is that InvocationCollector isn't stepping into extern {} blocks but I'll have to look at it.

cc #38356

[abonander]

Yep, this match block is missing a case for ItemKind::ForeignMod: https://github.com/rust-lang/rust/blob/master/src/libsyntax/ext/expand.rs#L961-L1033

@petrochenkov @nrc I can fix this one but it seems like a good mentoring bug since it might be that trivial or it might not be. I'm willing to mentor someone on it.

[mjbshaw]

If you're willing to mentor, I'm willing to take this one and be the mentee. I've been wanting to contribute to Rust but the compiler is so big and complex that I haven't been sure how to break into it.

[abonander]

Sure, I've been wanting to mentor for a while now. I don't have a great grasp on the overall compiler architecture but I like to think I know my way around this bit of the frontend.

The match block I linked is the likely culprit; this is the bit of the macros code that walks the syntax tree looking for invocations (bang!() and #[attribute]). It currently appears to be ignoring extern {} blocks (ast::ItemKind::ForeignMod) so a case for that needs to be added. It might be as simple as copying the case for ItemKind::Module but I'm not sure; it's probably the best place to start though.

If you need more active help we can meet up on IRC or something, I guess.

[abonander]

In fact it should be much simpler than ItemKind::Mod as you only need to fold over the individual items; you don't have to touch anything else that the Mod case does because it has to worry about resolution scopes and whether the module is declared inline or in another file.

[mjbshaw]

Thanks for those tips. I've been fiddling with this code for the past few days, trying to better understand how it works and what's needed. Unfortunately, with the long build times and my complete lack of experience in the Rust codebase, this is proving to be a bigger time commitment than I'm currently able to embrace. I'll let someone else take this issue.

[abonander]

@nrc Working on this, would we consider this a discrepancy in where macro attributes are allowed (insta-stable) or a new unstable usage?

[abonander]

After doing more digging, my concern right now is that it appears libsyntax isn't setup to accept macro expansions inside extern {} blocks at all, so I'm wondering if we even want to expand attribute macros there. I've got some WIP code at abonander@3e20e2f and it's a lot further-reaching than I'd like it to be but that's about the bare minimum to make attributes in extern {} blocks work.

[abonander]

@petrochenkov perhaps you'd like to weigh in

[nrc]

Working on this, would we consider this a discrepancy in where macro attributes are allowed (insta-stable) or a new unstable usage?

I think this should be behind a flag since I'm sure it's the kind of thing with unforeseen bugs that crop up. Ideally we can just put it behind the proc_macro feature, rather than creating a new one.

Why are extern blocks different to other blocks? I'd really hope that that sort of thing Just Works :-(

[abonander]

Why are extern blocks different to other blocks?

Presumably because they only support a subset of all item kinds. ForeignItem vs Item. That would have to be enforced in parsing the macro output.

In the process of enabling proc_macros here it wouldn't be a stretch to enable other macros as well, maybe all behind a macros_in_extern feature?

[abonander]

@petrochenkov What do you think of the previous comment?

Let me know if there's anyone else I should pull in on issues like this, I'm looking to knock out a bunch of them in quick succession.

[petrochenkov]

If mod m { call_macro!(); }, trait Tr { call_macro!(); }, impl [Tr for] Type { call_macro!(); } all work I see no reasons for extern "ABI" { call_macro!(); } not to work.

I suspect it's not implemented yet just because the demand wasn't high enough.

[abonander]

So if I do enable all macros in extern {} blocks should I put it behind a feature? The primary issue is that the kinds of items that macros can emit in this context are restricted, and that might surprise some people.

[petrochenkov]

So if I do enable all macros in extern {} blocks should I put it behind a feature?

Citing nikomatsakis, "Basically every time I've skipped a feature gate I've
regretted it."

The primary issue is that the kinds of items that macros can emit in this context are restricted, and that might surprise some people.

This is equivalent to something that already exist:

macro make_struct() {
    struct S;
}

trait Tr {
    make_struct!(); // ERROR expected one of `const`, `extern`, `fn`, `type`, or `unsafe`, found `struct`
}

fn main() {}

[petrochenkov]

Also see #48137 about how treatment of module/trait/impl/foreign items in macros can be made more uniform.