-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Stabilize built-in attribute macro #[cfg_eval]
#87221
Conversation
We discussed this in today's @rust-lang/lang meeting. Sorry for the delay in processing this. The general sentiment in the meeting was that we were uncertain about putting Whether as an opt-in (for current editions) or an opt-out (for a future edition that defaults to it), either way we felt like this should be tied to the proc macro and not to the call site. We know that people want eager evaluation for macros, but it'd be helpful to understand what the use case is for determining this at the call site. |
#[cfg(foo)] // expanded first
#[bar] // expanded second
#[cfg(baz)] // expanded third
struct S {
#[xyz] // expanded fourth
#[cfg(abc)] // expanded fifth
field: u8,
} In that case there's no language way to expand attributes out-of-order, we can only ask the leftmost attribute to process its input and output the processed token stream. If we want deep cfg-expansion as processing, then If we are ok with |
I am nervous about the piecemeal stabilization of changes here. It feels a bit directionless to me. I am wondering whether we could start an initiative to draft up and document the considerations, as well as the overall goal we are reaching for. For example, latent in @petrochenkov's comment seems to be an assumption that "outer-to-inner" processing is what we want-- this may be true, it makes some sense to me, but I'd like to see that evaluated against a set of use cases or something. |
Put another way, I think that @petrochenkov and others likely have put quite a bit of thought into what they'd like to see, but I feel like we haven't captured those thoughts in a way that I can readily read and understand. |
How will this work with two macros, one which cares about cfg values and the other one which needs them evaluated. Let's assume the proc-macros do not alter the tokenstream and simply return the input. So right now, the order of the attributes would not matter. #[no_cfg]
#[cares_about_cfg]
struct S {}
// or
#[cares_about_cfg]
#[no_cfg]
struct S {} In the first case, will the |
There's some additional context to this, but not too much. Multiple attributes on the same target were always resolved and expanded in left-to-right order in general, but there were a few exceptions. In the last couple of years I've been working on eliminating such exceptions, starting from hacks like #63248, and finishing with turning Right now the only remaining attributes that are expanded out-of-order are // `cfg_attr` is evaluated first, and written second so the lint is reported
// however, we cannot reorder the attributes because `my_trait_helper` is not in scope before `#[derive(MyTrait)]`
// but we can use `cfg_eval` to both evaluate `cfg_attr` and keep relative order of `MyTrait` and `my_trait_helper`
#[derive(MyTrait)]
#[cfg_attr(my_feature, my_trait_helper)]
struct S; Left-to-right expansion makes everything simpler starting from mental model, and ending with compiler implementation, including token collection for attribute macros. |
Note that |
This example took me a second to understand. Let me read it back, cause I guess I get it now: #[derive(MyTrait)]
#[cfg_attr(my_feature, my_trait_helper)]
struct S; The problem here is that derive excepts to be a "fully configured" input, right? But if we move that #[my_trait_helper]
#[derive(MyTrait)]
struct S; and that is broken. So we put |
Alternatively, it seems like a limited form of "eager expansion", right? i.e., if the proc macro API had some way to say "expand attributes matching some filter that you see in this list", then you could implement it this as a procedural macro. |
I ask because: I agree that left-to-right is desirable; but I'm also not super keen on having to write I've traditionally been resistant to eager expansion, but it does seem kind of inevitable that we'll have attributes that do want to do something like it. |
No, #[derive(MyTrait)]
#[cfg_attr(my_feature, my_trait_helper)]
struct S; is perfectly ok. The problem is purely with the lint, in the world with #[test]
#[cfg(FALSE)] // WARNING: `cfg` is written after `test`, but expanded before `test`, please reorder
fn foo() {} I guess #[allow(out_of_order_expansion)]
#[derive(MyTrait)]
#[cfg_attr(my_feature, my_trait_helper)]
struct S; would work too, since it's indeed a false positive, and reordering is not the right thing to do here. |
Eager expansion in general should be able to resolve paths, and deal with the "path cannot be resolved yet" situations. |
So, I think for a proc macro that wants to mostly add code rather than to modify the item it would be ideal to be able to use both the configured version of the input for adding new code and the original version of it for re-emitting. This functionality can neither be provided by |
That seems useful for the example I gave, and only by the description I gave, a derive macro likely works better. I don't know of any specific pair of proc-macros which would behave like this. I mainly wanted to add to this point by @joshtriplett, by showing that some situations might still arise where the caller needs to know if the proc-macro evaluates cfg-attributes.
|
We discussed this in our @rust-lang/lang meeting on 2021-10-12. @petrochenkov we wanted to clarify something and make sure we understand it. Based on your OP, it seems like you think the right future is one where "cfg happens by default", before any procedural macros run. But unfortunately before we could do that switch, there would be a need to have some kind of opt-out? I'm not quite sure, then, why we are adding a opt-in, and I'm not quite sure where the idea of a lint that encourages reordering things fits in here, especially since that reordering is not (ultimately) expected to matter. |
I'm not sure in this.
So 3 cases against 1 for not doing the configuration by default, or at least providing a way to observe the unconfigured version of the input together with the configured one.
Because it will be useful for at least 3 years, regardless of whether we switch the defaults later or not. |
I'm going to close this for now and first land the refactoring parts of #83354, and implement the lint discussed above. |
Built-in attribute macro
#[cfg_eval]
is used for eagerly expanding all#[cfg]
and#[cfg_attr]
attributes in its input ("fully configuring" the input).The effect is identical to effect of
#[derive(Foo, Bar)]
which also fully configures its input before passing it to macrosFoo
andBar
, except thatcfg_eval
is supported on any code fragments supporting macro attributes, unlikederive
.Example:
The main alternative to this attribute is to automatically fully configure items before expanding any attributes.
That alternative was implemented as an experiment in #85073.
Crater showed that there are indeed attributes that want to do some custom
cfg
processing (#85073 (comment)) which are broken by this change, so we need some kind opt-out from eager configuration, but it's not clear how that opt-out should look like.I suggest considering switching to this alternative for the next edition (~Rust 2024), but leave it alone for now.
This PR is somewhat related to #87220, but I don't think they are blocking each other.
Closes #82679
r? @Aaron1011