Macros bypass other feature gates

[brson]

This uses the log_syntax! macro without the log_syntax feature gate:

#[feature(macro_rules)];
// This should be required
// #[feature(log_syntax)];

macro_rules! hey ( () => { log_syntax!() } )

fn main() {
    hey!()                                                                                         
}

Nominating.

[alexcrichton]

This will likely require an overhaul of the feature_gate infrastructure to process the AST after expansion, not before. You then run into fun things like use std::prelude::*; shouldn't be gated and neither should #[phase(link, syntax)] extern mod std;

[alexcrichton]

Hm, and that won't necessarily work because macro invocations no longer exist in the ast after expansion. This may be a little though.

[sfackler]

It's a bit hacky, but we can always just run feature gating before and after expansion like we already do with cfg processing.

[sfackler]

It'd require a bit of trickyness since std-injection uses #[phase(syntax, link)]. Maybe it's finally time to get #[feature(..)] working in places other than the crate root.

[glaebhoerl]

I feel like the check should happen where the macro is declared, not where it's used. (Currently neither happens of course, which is definitely wrong.)

[lilyball]

Feature gating after expansion means that clients of your macro will have to turn on feature gates even though they aren't using those features themselves. I feel like clients of a macro should not have to suffer the burden of dealing with this, especially as they only way they'll even know what gates to turn on is to compile, fail, and read the error message.

Is there any way the feature gating machinery could read the macro definition and see the problem there?

[sfackler]

That would be hard for macro_rules macros and impossible for procedural macros. If #[feature(..)] were changed so that it could be attached to anything, the definition could be adjusted to

macro_rules! hey ( () => { #[feature(log_syntax)] log_syntax!() } )

and then rustc could run feature gating before and after expansion. It'd clutter up macros a bit though.

[lilyball]

Could the macro embed information about what feature gates are in effect at its definition, and then mark the generated AST with that same feature gate information?

[sfackler]

That'd be doable, but it'd have to be a manual opt-in thing by the macro author.

[sfackler]

It'd also preferably be mixed with properly scoped #[feature] support.

[pnkfelix]

This definitely blocks unfeature-gating macro_rules! for 1.0.

But it is technically not otherwise a 1.0 blocker.

Yes it seems like enough of a trap to assign P-high; so that's what we're doing.

[alexcrichton]

Nominating, I believe our story around macros has changed in the meantime.

[emberian]

I have a few ideas for this that involve parsing the rhs of each arm in the macro_rules as everything it could be expanded to, and feature gating that output. Pretty sketch, really. The alternative is feature gating everything after macro expansion, and "somehow marking" the std macros as "ok to expand to unstable code". Or perhaps not expanding to unstable code in the std macros! Not sure if that's feasible or not without reviewing them (which I haven't done yet), but I imagine format_args will be the sitckler.

[alexcrichton]

I've been thinking recently as well that we should just feature gate the entire expansion of the macro, regardless of where it came from. The parts that may be difficult to get around:

• std::fmt has tons of fiddly bits we'd have to mark #[stable] as a result.
• std::thread_local may inject #[thread_local]
• ... ?

I'm not actually sure that the list is too long, so we may be able to surmount most of it!

One example of we really need to check expansions is that the select! macro is super unstable, but you don't get warnings for using it today.

[emberian]

If we can get away with it I still think we should gate the macro definition as well, so as to not surprise unsuspecting users with libraries that are supposedly stable.

[alexcrichton]

Agreed!

[pnkfelix]

assigning 1.0 milestone; there are real problems that we need to resolve here, but we can also claim "just a bug" in many respects

[pnkfelix]

we believe this to be fixed now. At least, the playpen warned now on this updated example:

#![feature(macro_rules)]
// This should be required
// #[feature(log_syntax)];

macro_rules! hey { () => { log_syntax!() } }

fn main() {
    hey!()                                                                                         
}