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

Process cfg_attr during expansion #22250

Closed
kmcallister opened this issue Feb 13, 2015 · 15 comments · Fixed by #33706
Closed

Process cfg_attr during expansion #22250

kmcallister opened this issue Feb 13, 2015 · 15 comments · Fixed by #33706
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-syntaxext Area: Syntax extensions

Comments

@kmcallister
Copy link
Contributor

macro_rules! borked {
    () => {
        #[derive(Debug)] pub struct Good;
        #[cfg_attr(not(test), derive(Debug))] pub struct Bad;
    }
}

borked!();

fn main() {
    println!("{:?}", Good);
    println!("{:?}", Bad);
}

produces

foo.rs:12:22: 12:25 error: the trait `core::fmt::Show` is not implemented for the type `Bad` [E0277]  
foo.rs:12     println!("{:?}", Bad);  
                               ^~~  

I think what happens is the following:

  • The first cfg stripping pass runs. At this point the macro body is just a token tree, and there's no way for cfg stripping to work out what # [ cfg_attr ( not ( test ) ... means.
  • Syntax expansion runs. The macro emits two items. One has a derive attribute, which matches a Decorator in the syntax environment, so that extension also runs. There's no extension named cfg_attr so that attribute has no effect on expansion.
  • The post-expansion cfg stripping pass runs. The second attribute turns into #[derive(Debug)] but not in time for syntax expansion to notice it.

Previously, cfg_attr was a syntax extension, which had numerous drawbacks, but handled this case correctly.

The somewhat obvious fix is to re-run cfg_attr processing on the AST subtree produced by every macro expansion, but I'd like to think about this more. I'd also like to know whether it's something people often run into in practice.

Was mentioned on #19372. cc @sfackler, @SergioBenitez

@kmcallister kmcallister added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-syntaxext Area: Syntax extensions labels Feb 13, 2015
@mathall
Copy link

mathall commented Feb 13, 2015

I can't answer whether it's something people often run into in practice, but I have run into this bug myself.

@kmcallister
Copy link
Contributor Author

Interesting; can you tell me more about the circumstances?

@mathall
Copy link

mathall commented Feb 13, 2015

I'm using the bitflags macro which generates a struct and allows me to declare decorators for that struct. Decorators such as #[cfg_attr(not(test), derive(Debug))].

mathall added a commit to mathall/rim that referenced this issue Feb 22, 2015
vi added a commit to vi/mkv.rs that referenced this issue Aug 24, 2015
@DanielKeep
Copy link
Contributor

I've just run into this. I'm writing some FFI bindings that involve gating pretty much every function and type. As such, I wrote the following to attach attributes to groups of items:

macro_rules! attr_block {
    ($(#[$attrs:meta])* {}) => {};

    (
        $(#[$attrs:meta])* {
            $it:item $($tail:tt)*
        }
    ) => {
        $(#[$attrs])*
        $it

        attr_block! { $(#[$attrs])* {$($tail)*} }
    };
}

All is well until I started adding #[derive(..)]s, at which point the wheels fell off.

@DanielKeep
Copy link
Contributor

This also means that custom_derive! is fundamentally incompatible with #[cfg(..)], assuming you're using any built-in derives. As far as I can see, there's no way to work around this.

@durka
Copy link
Contributor

durka commented Dec 9, 2015

I have run into this in practice as well, I was trying to write a similar macro to @DanielKeep. I settled on making an inner module, but it's not a perfect solution in many ways. It would come up with crates like cfg-if as well.

One really vexing thing about this bug is it's a silent failure. At least rustc should output a warning when it drops a cfg/cfg_attr without interpreting it.

@Kimundi
Copy link
Member

Kimundi commented Dec 10, 2015

I'm currently looking into how to possibly fix this - it would likely involve restructuring how the compiler processes the config and expansion phases.

@Kimundi
Copy link
Member

Kimundi commented Dec 10, 2015

cc @nrc

@Amanieu
Copy link
Member

Amanieu commented Jan 13, 2016

I just ran into this issue. Here is basically what I was trying to do:

macro_rules! foo {
    () => {
        #[cfg(feature = "nightly")]
        const fn bar() {}
        #[cfg(not(feature = "nightly"))]
        fn bar() {}
    };
}
foo!();

This fails with const fn is unstable even though the const fn should have been eliminated by the #[cfg].

@tikue
Copy link
Contributor

tikue commented Apr 25, 2016

@Amanieu just hit the same issue.

@Kimundi
Copy link
Member

Kimundi commented Apr 28, 2016

Hm, totally forgot that I was working on this at some point... If anyone had been waiting on me, I think I'd have to start work on this from scratch.

@jseyfried
Copy link
Contributor

This was fixed in #33706.

@durka
Copy link
Contributor

durka commented May 29, 2016

Awesome, I didn't expect this to actually get fixed!
On May 28, 2016 21:02, "Jeffrey Seyfried" notifications@github.com wrote:

This was fixed in #33706 #33706.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#22250 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAC3nw_uG11kkqLPkWu4hOiD8SmHyG4gks5qGOWOgaJpZM4Df3US
.

@Kimundi
Copy link
Member

Kimundi commented May 30, 2016

Nice!

@vi
Copy link
Contributor

vi commented Sep 11, 2016

Shall this issue be closed?

joshtriplett added a commit to joshtriplett/error-chain that referenced this issue Dec 25, 2016
error_chain uses attributes on statements, stablized in Rust 1.13.
Factor them out of functions in the non-backtrace codepath, making it
possible to build a crate that uses error_chain with Rust 1.10.

The backtrace feature still requires Rust 1.13 for attributes on
statements, due to the impl_extract_backtrace macro, which needs to emit
a function with one block for each linked error type, some of which may
have attributes on them.  And any code using compile-time conditionals
in attributes on variants within an error_chain! invocation (which
includes many of the tests and examples) will produce an error due to
rust-lang/rust#22250 (fixed in Rust 1.11).
But a crate using error_chain with default-features = false and avoiding
such compile-time conditionals will now compile.
joshtriplett added a commit to joshtriplett/error-chain that referenced this issue Dec 25, 2016
- Avoid using attributes on statements; use them on items instead.

- Change a test to use std::env::VarError instead of std::fmt::Error,
  because the latter didn't impl Error until Rust 1.11.

- Add an empty block after invocations of `error_chain!` inside
  functions, to work around rust-lang/rust#34436
  (fixed in Rust 1.11).

- Replace a single instance of `?` with `try!`.

This still fails to work with 1.10 due to
rust-lang/rust#22250 .  Because of that issue,
an invocation of `#[derive(Debug)]` inside a macro doesn't take cfg(...)
attributes into account, and fails to compile due to referencing enum
variants that don't exist.  Posted for reference only.
joshtriplett added a commit to joshtriplett/error-chain that referenced this issue Dec 25, 2016
- Avoid using attributes on statements; use them on items instead.

- Change a test to use std::env::VarError instead of std::fmt::Error,
  because the latter didn't impl Error until Rust 1.11.

- Add an empty block after invocations of `error_chain!` inside
  functions, to work around rust-lang/rust#34436
  (fixed in Rust 1.11).

- Replace a single instance of `?` with `try!`.

This still fails to work with 1.10 due to
rust-lang/rust#22250 .  Because of that issue,
an invocation of `#[derive(Debug)]` inside a macro doesn't take cfg(...)
attributes into account, and fails to compile due to referencing enum
variants that don't exist.  Posted for reference only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-syntaxext Area: Syntax extensions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants