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

Consider expanding through an indirect macro #25

Closed
gnzlbg opened this issue Sep 20, 2019 · 8 comments
Closed

Consider expanding through an indirect macro #25

gnzlbg opened this issue Sep 20, 2019 · 8 comments

Comments

@gnzlbg
Copy link

gnzlbg commented Sep 20, 2019

See rust-lang/rfcs#2523 (comment), 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
Copy link
Member

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
Copy link
Author

gnzlbg commented Sep 20, 2019

@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
Copy link
Author

gnzlbg commented Sep 20, 2019

@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
Copy link

@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
Copy link

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

@alexcrichton
Copy link
Member

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

This was referenced Sep 23, 2019
@gnzlbg
Copy link
Author

gnzlbg commented Sep 23, 2019

FWIW @ogoffart implementation works correctly for libc at least.

@Lokathor
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants