Allow passing expr metavariable as cfg predicate

[Jules-Bertholet]

View all comments

This PR allows expanding expr metavariables inside the configuration predicates of cfg and cfg_attr invocations.
For example, the following code will now compile:

macro_rules! mac {
    ($e:expr) => {
        #[cfg_attr($e, inline)]
        #[cfg($e)]
        fn func() {}

        #[cfg(not($e))]
        fn func() {
            panic!()
        }
    }
}


mac!(any(unix, feature = "foo"));

There is currently no macro_rules fragment specifier that can represent all valid cfg predicates. meta comes closest, but excludes true and false. By fixing that, this change makes it easier to write declarative macros that parse cfg or cfg_attr invocations, for example #146281.

@rustbot label T-lang needs-fcp A-attributes A-cfg A-macros

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[petrochenkov]

Just as an update, after #124141 we effectively no longer have nonterminal tokens in the grammar (*).
At parsing time a pasted expr metavariable is not some NtExpr token, it's just a token stream in invisible parentheses.
If that token stream from expr parses as a cfg predicate then it can be used as a cfg predicate, if it parses as a type or pattern, then it can be used as a pattern as well.
And on the other hand, if a token stream from some other matcher like ty or pat parses as a cfg predicate, then it can be used as a cfg predicate.
In general, the matcher kind is only relevant during macro LHS matching, not during RHS parsing (**).

I hoped that some time after #124141 either @nnethercote or me relax these rules in many cases, when there are no backward compatibility concerns at least, and pass the change through lang team, but it didn't happen yet.
(In particular, tokens from any matchers should be parse-able in expr, ty and pat positions, there should be no compatibility issues there.)

A number of current hacks like accepting impl $ty for Type, or what this PR suggests, are just special cases of that strategy.

@Jules-Bertholet You could very well start the same process, just from a different point and accept any token streams that look like cfg predicates as cfg predicates, from any matchers, not just expr.

(*) Or we technically have, but only for compatibility and to avoid extending the language without the lang team process.
(**) Again, unless there are backward compatibility issues.

[petrochenkov]

As for the current implementation, the new expr-accepting logic is used in parse_meta_item and the new attr parsing infra, so it seems more than just cfg predicates specifically, but I'm not sure what else exactly is affected.

[joshtriplett]

This seems reasonable to me. It allows all :expr fragments here, and not all of those will subsequently parse, but that seems okay.

@rfcbot merge

[rust-rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[traviscross]

@rfcbot reviewed

[tmandry]

@rfcbot reviewed

[rustbot]

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Jules-Bertholet]

@rustbot ready

This should now be confined to affecting cfg predicates only.

[petrochenkov]

r? @JonathanBrouwer
Could you double check this? The new attribute parsing infra is impenetrable.
The expr fragments are now supposed to parsed in cfg predicates, and only in cfg predicates.

(Why does it all converge to parse_attr_item? cfg predicates are not attr items at all...)

[Jules-Bertholet]

@rustbot ready

[JonathanBrouwer]

@bors r+ rollup

View changes since this review

[rust-bors]

📌 Commit ab36d50 has been approved by JonathanBrouwer

It is now in the queue for this repository.