parser warning "macro is experimental" interacts badly with module-level cfg

[RalfJung]

Given code like this

#![feature(decl_macro)]
#![cfg(someflagthatisnotset)]

pub macro mymacro() {}

what I expect should happen is that this works fine, since the feature flag is set so the parser should accept parsing the pub macro, and also the file is anyway empty after cfg expansion. However, I get a warning instead:

warning: `macro` is experimental
 --> src/lib.rs:4:1
  |
4 | pub macro mymacro() {}
  | ^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: [see issue #39412 <https://github.com/rust-lang/rust/issues/39412>](https://github.com/rust-lang/rust/issues/39412) for more information
  = help: [add `#![feature(decl_macro)]`](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#) to the crate attributes to enable
  = warning: unstable syntax can change at any point in the future, causing a hard error!
  = note: for more information, [see issue #65860 <https://github.com/rust-lang/rust/issues/65860>](https://github.com/rust-lang/rust/issues/65860)

It looks like the cfg does disable the feature flag, but does not disable parsing of the rest of the file. That is a problem, since it means it is impossible to cfg-out an entire file that contains experimental syntax. I thought I could fix this by ordering the cfg after the feature, but as the example shows that does not help.

This probably started happening with #99935.
Cc @CAD97 @petrochenkov

[CAD97]

Two bits:

• Doesn't #![feature] have to be at the crate root?
• You can #[cfg] out parsing of a module, but it has to be on the mod statement.

#[cfg] are always processed after parsing, so I believe the compat warning is correct here. At least, doing this same thing with properly gated syntax causes an error. For example,

#![cfg(FALSE)]

fn test() {
    do yeet;
}

is a hard error for the unstable/experimental syntax. The unstable syntaxes that are only warnings when cfgd out is more an unfortunate happenstance, since this makes them at least somewhat syntax-stable. The ones uplifted to warnings by #99935 were ones crater said were essentially not being used at the time it was originally tested.

[RalfJung]

Doesn't #![feature] have to be at the crate root?

Yes. But there are cases where one wants the entire crate to be cfg'd out, and that's where I ran into this. (See #104634 for the work-around. This is related to this cfg.)

You can #[cfg] out parsing of a module, but it has to be on the mod statement.

I assume this only works for mod file; though, not mod inline { ... }?

#[cfg] are always processed after parsing, so I believe the compat warning is correct here. At least, doing this same thing with properly gated syntax causes an error. For example,

The thing is, this should work, since I am setting the feature gate

#![feature(yeet_expr)]
#![cfg(FALSE_DOES_NOT_EXIST_IN_CFG)]

fn test() {
    do yeet;
}

If the parser has to parse all the things that are being cfg out, it should also take into account the feature gates. It is problematic that it happily ignores cfgd feature gates but then complains about other disabled things.

[petrochenkov]

@RalfJung
This was discussed in detail in #65860, you even participated in the discussion.

[RalfJung]

I vaguely remember some discussions, but this seems slightly different? I am running in a nightly compiler either way. I am not asking that a stable compiler accept such a file. I don't think there is a stability risk here. The problem is that there is no way to set a feature flag in a way that the parser will always take it into account. Hm, re-reading that old issue it does sound pretty much the same. But I don't think there is a stability risk here -- I am totally fine with the stable compiler emitting an error here.

If the parser goes over the entire file, no matter the cfg, then why can't it take into account all feature, no matter the cfg? The current behavior does not seem coherent. I wouldn't complain if it parsed all things ignoring cfg and applied all feature gates to the parser, ignoring cfg.

[petrochenkov]

why can't it take into account all feature, no matter the cfg?

Ah, this part, it may be doable, yes.

[RalfJung]

If a #![cfg] only erased the attributes that come after it, that would also work. But it seems to apply "backwards in code" so there's no place to put a feature that the parser would honor if there's a crate-level cfg disabling things.

[shepmaster]

I have something similar. In SNAFU, I have a number of crates that are only for testing purposes. I was experimenting with yeet:

Cargo.toml

[package]
name = "repro"
version = "0.1.0"
edition = "2021"

[dependencies]

src/lib.rs

#![cfg(test)]
#![feature(yeet_expr)]

fn x() -> Result<(), i32> {
    do yeet 0;
}

cargo +nightly test isn't happy:

error[E0658]: `do yeet` expression is experimental
 --> src/lib.rs:5:5
  |
5 |     do yeet 0;
  |     ^^^^^^^^^
  |
  = note: see issue #96373 <https://github.com/rust-lang/rust/issues/96373> for more information
  = help: add `#![feature(yeet_expr)]` to the crate attributes to enable

Is this the same case, or should I open a new issue?

[CAD97]

It's the same issue. I agree that it's reasonable to apply at least the #![feature] which occur before a #![cfg].

If they were outer attributes,

#[feature(...)]
#[cfg(...)]
crate {
}

the #[feature] attribute would be logically evaluated before the #[cfg] attribute is evaluated, and thus the #[cfg] shouldn't suppress the effect of the #[feature].

[RalfJung]

@CAD97 yes that would resolve the issue for me. :)

[RalfJung]

I guess that would technically be a breaking change since this will currently compile on stable

#![feautre(...)]
#![cfg(notathing)]

However, this generates an empty crate, so... doesn't seem terribly likely? The cfg can be ordered up to make this stable-compatible. (And of course then the parser error do apply, and that is indeed their entire point.)

[petrochenkov]

Fixed in #108221.

[petrochenkov]

It turns out stuff breaks if features are considered for fully unconfigured crates #108221 (comment).

[petrochenkov]

the #[feature] attribute would be logically evaluated before the #[cfg] attribute is evaluated, and thus the #[cfg] shouldn't suppress the effect of the #[feature].

This is not how it currently works in other places in the language, the attribute expansion algorithm always picks cfg and cfg_attr attributes first for expansion, and then proceeds to attribute macros and inert attributes.

Things also break if we change the order to be fully left-to-right (I tried it, it would be nice to have this kind of simplification - #83331).

I guess we could make an exception and change the order for the specific list of "very early" attributes, but I'm not sure it's good idea, maybe we should just leave this papercut alone.

[petrochenkov]

maybe we should just leave this papercut alone

On the other hand it then becomes impossible to fully unconfigure a crate if it directly contains unstable syntax, which also doesn't seem right.

I'll try to implement this left-to-right amendment for a second crater run, let's see what happens.
It's a hack, but these "very early" attributes are already hacky enough due to being required before the main expansion pass.

[RalfJung]

On the other hand it then becomes impossible to fully unconfigure a crate if it directly contains unstable syntax, which also doesn't seem right.

Yeah, that is what prompted this issue. It can be worked around (by moving all unstable syntax our of the lib.rs file) but that is somewhat unsatisfying. Though given backwards compatibility constraints, none of the hacks that make this possible are really satisfying either...

I'll try to implement this left-to-right amendment for a second crater run, let's see what happens.

Sounds plausible. Another option might be to special-case only feature. AFAIK none of the other attributes cause problem with fully unconfiguring a crate?

[petrochenkov]

Next attempt - #110141.

[RalfJung]

Hm, this had some odd fallout in miri-test-libstd, not sure what is going on there
https://github.com/rust-lang/miri-test-libstd/actions/runs/5233119090/jobs/9448362774

[RalfJung]

Okay it's mostly resolved by reordering as one would expect: #112535.

But one strange lint remains:

warning: unknown lint: `fuzzy_provenance_casts`
   --> std_miri_test/../library/std/src/lib.rs:610:1
    |
610 | #[allow(dead_code, unused_attributes, fuzzy_provenance_casts)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: the `fuzzy_provenance_casts` lint is unstable
    = note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
    = help: add `#![feature(strict_provenance)]` to the crate attributes to enable
    = note: `#[warn(unknown_lints)]` on by default

@petrochenkov looks like it is processing that allow(...) deep inside the crate (it is attached to a mod) but not processing the feature(strict_provenance) at the crate root?