rust-lang / cfg-if

A if/elif-like macro for Rust #[cfg] statements
Apache License 2.0
557 stars 40 forks source link

Consider expanding through an indirect macro #25

Closed gnzlbg closed 5 years ago

gnzlbg commented 5 years ago

See https://github.com/rust-lang/rfcs/pull/2523#issuecomment-533466004, where @ogoffart mentions that:

I suppose it would not make sense to relax the parsing rules of items that are disabled by #[cfg]

So one must wrap the item inside a macro such as cfg-if!{...} but which would not expand the items for the disabled configuration. Something like this should work: #[cfg($cfg)] identity!{ $($body)* }.

Currently in libc we can't write:

cfg_if! {
    if #[cfg(foo)] {
        #[repr(align(16))] struct F;
    }
}

because older libsyntax versions chuckle on the align(16) (https://rust.godbolt.org/z/eZYANH). It would be nice if cfg_if! could be improved with such an indirect macro approach to make that work.

alexcrichton commented 5 years ago

I'm not personally too keen on adding compatibility code to this crate to be compatible so far back, so I'm going to go ahead and close this. If there's a PR though and it's relatively self-contained I'd be happy to review! I would prefer to not have an outstanding open issue for this, though.

gnzlbg commented 5 years ago

@alexcrichton Note that async fn, const fn, pub(crate), cfg(version(1.35)), ... all run into this problem. You can't use cfg_if! to gate these because it doesn't prevent parsing on the code it gates. So it isn't about being compatible with code "far back", it is about being compatible with last months toolchain. Every time the syntax grows, this issue happens.

gnzlbg commented 5 years ago

@ogoffart FWIW I tried your approach here with async fn: https://rust.godbolt.org/z/ULb3I1 but gating these behind an identity! macro did not work.

ogoffart commented 5 years ago

@alexcrichton right, repr(align(16)) was just an example, but this is true for anyone trying to use features to gate code using grammar that is only available in a newer version of rust.

@gnzlbg right: becuase the the code need to be parsed as an item to be mached b the $:item in the macro. Using tt instead fix this problem. This might have other side effects, i'm not sure. https://rust.godbolt.org/z/9Wvn7v

ogoffart commented 5 years ago

btw, you don't really need cfg_if, the identity macro alone does the trick.

alexcrichton commented 5 years ago

Oh nice @ogoffart! Want to send a PR with that? That looks like a small enough change and also looks to enable cfg_if in an expression position as well

gnzlbg commented 5 years ago

FWIW @ogoffart implementation works correctly for libc at least.

Lokathor commented 5 years ago

Using the identity! macro makes the cfg-if tests fail because when cfg_if! expands it can't find the identity! macro. However, converting it to an "internal macro" like with @__apply makes it all seem to work out.